From 230dbb0e0ce7267d63cd440df900c56193736094 Mon Sep 17 00:00:00 2001 From: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> Date: Sat, 7 Nov 2026 02:32:10 +0100 Subject: [PATCH 1/4] fix: edit and resubmit returned requests (#42) Reopen returned drafts with their values and revision reason. Add owner-only versioned editing/resubmission, shared validation, balance/overlap self-exclusion, transactional concurrency guards, employee names, synchronized API contracts and regression/manual acceptance coverage. Co-authored-by: patrickschiller <39913537+patrickschiller@users.noreply.github.com> Signed-off-by: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> --- .../src/api/request-revision.e2e.spec.ts | 452 ++++++++++++++++++ apps/api/openapi.json | 386 ++++++++++++++- .../app/accounts/vacation-balance.service.ts | 13 +- .../leave-allowances.service.ts | 16 +- .../src/app/requests/requests.controller.ts | 45 +- apps/api/src/app/requests/requests.dto.ts | 121 +++-- apps/api/src/app/requests/requests.service.ts | 378 +++++++++------ apps/web/src/api/client.ts | 19 + apps/web/src/api/generated.ts | 199 +++++++- apps/web/src/app/i18n.tsx | 18 + .../web/src/routes/AdminRequestsPage.spec.tsx | 52 ++ apps/web/src/routes/AdminRequestsPage.tsx | 4 +- apps/web/src/routes/RequestsPage.spec.tsx | 138 +++++- apps/web/src/routes/RequestsPage.tsx | 194 +++++++- docs/ISSUE_FIXES_MANUAL_TEST.de.md | 209 ++++++++ 15 files changed, 1994 insertions(+), 250 deletions(-) create mode 100644 apps/api-e2e/src/api/request-revision.e2e.spec.ts create mode 100644 apps/web/src/routes/AdminRequestsPage.spec.tsx create mode 100644 docs/ISSUE_FIXES_MANUAL_TEST.de.md diff --git a/apps/api-e2e/src/api/request-revision.e2e.spec.ts b/apps/api-e2e/src/api/request-revision.e2e.spec.ts new file mode 100644 index 0000000..177ea93 --- /dev/null +++ b/apps/api-e2e/src/api/request-revision.e2e.spec.ts @@ -0,0 +1,452 @@ +import { EmployeesService } from '../../../api/src/app/employees/employees.service'; +import { + createTestApp, + login, + seedEmployee, + seedLeaveAllowance, + type TestContext, +} from '../support/test-app'; + +const YEAR = 2026; +const FROM = '2026-05-04T00:00:00.000Z'; +const TO = '2026-05-08T00:00:00.000Z'; + +describe('Returned requests — owner revision and resubmission (#42)', () => { + let ctx: TestContext; + let owner: { id: string; token: string }; + let manager: { id: string; token: string }; + let other: { id: string; token: string }; + let hr: { id: string; token: string }; + + beforeAll(async () => { + ctx = await createTestApp(); + }); + afterAll(async () => { + await ctx.close(); + }); + beforeEach(async () => { + await ctx.reset(); + async function person( + personalNo: string, + role: 'Employee' | 'Manager' | 'HRAdmin', + managerId?: string, + ) { + const e = await seedEmployee(ctx.prisma, { + personalNo, + firstName: personalNo, + lastName: 'Example', + email: `${personalNo}@test.local`, + role, + managerId, + }); + return { id: e.id, token: await login(ctx.http, e.email) }; + } + manager = await person('manager', 'Manager'); + owner = await person('owner', 'Employee', manager.id); + other = await person('other', 'Employee', manager.id); + hr = await person('hr', 'HRAdmin'); + await seedLeaveAllowance(ctx.prisma, owner.id, YEAR, 5); + }); + + async function create(type = 'Vacation', substituteId?: string) { + const r = await ctx.http + .post(type === 'Vacation' ? '/api/requests/vacation' : '/api/requests') + .set('Authorization', `Bearer ${owner.token}`) + .send({ + employeeId: owner.id, + type, + from: FROM, + to: TO, + substituteId, + reason: 'Original', + }) + .expect(201); + return r.body; + } + async function returned(type = 'Vacation') { + const r = await create(type); + const response = await ctx.http + .post(`/api/requests/${r.id}/return`) + .set('Authorization', `Bearer ${manager.token}`) + .send({ actorId: manager.id, note: 'Bitte kürzen' }) + .expect(201); + return response.body; + } + function values(r: { updatedAt: string }, extra = {}) { + return { + from: FROM, + to: TO, + reason: 'Korrigiert', + expectedUpdatedAt: r.updatedAt, + ...extra, + }; + } + function patch(id: string, token = owner.token) { + return ctx.http + .patch(`/api/requests/${id}`) + .set('Authorization', `Bearer ${token}`); + } + function submit(id: string, token = owner.token) { + return ctx.http + .post(`/api/requests/${id}/resubmit`) + .set('Authorization', `Bearer ${token}`); + } + + it('manager return → save → resubmit retains ID/history, excludes self, recalculates halves and resets assignments', async () => { + const r = await returned(); + expect(r).toMatchObject({ + workflowState: 'Draft', + status: 'Submitted', + employeeName: 'owner Example', + decisionNote: 'Bitte kürzen', + }); + const saved = await patch(r.id) + .send(values(r, { halfDayStart: true })) + .expect(200); + expect(saved.body).toMatchObject({ + id: r.id, + workflowState: 'Draft', + currentApproverId: null, + approverId: null, + calculatedDays: 4.5, + }); + expect(saved.body.updatedAt).not.toBe(r.updatedAt); + // Old decision data cannot leak into the new approval cycle. + await ctx.prisma.request.update({ + where: { id: r.id }, + data: { + approverId: manager.id, + hrConfirmedAt: new Date(), + decidedAt: new Date(), + cancelledAt: new Date(), + substituteAcceptedAt: new Date(), + }, + }); + const fresh = await ctx.http + .get(`/api/requests/${r.id}`) + .set('Authorization', `Bearer ${owner.token}`) + .expect(200); + const result = await submit(r.id) + .send(values(fresh.body, { halfDayStart: true })) + .expect(201); + expect(result.body).toMatchObject({ + id: r.id, + workflowState: 'PendingManager', + currentApproverId: manager.id, + calculatedDays: 4.5, + approverId: null, + substituteAcceptedAt: null, + hrConfirmedAt: null, + cancelledAt: null, + decidedAt: null, + decisionNote: null, + }); + expect(await ctx.prisma.request.count()).toBe(1); + const events = await ctx.prisma.requestEvent.findMany({ + where: { requestId: r.id }, + orderBy: { occurredAt: 'asc' }, + }); + expect(events.map((e) => e.kind)).toEqual([ + 'Submitted', + 'Returned', + 'Resubmitted', + ]); + expect(events[1].note).toBe('Bitte kürzen'); + const listed = await ctx.http + .get(`/api/requests?currentApproverId=${manager.id}`) + .set('Authorization', `Bearer ${manager.token}`) + .expect(200); + expect(listed.body[0].employeeName).toBe('owner Example'); + }); + + it('initializes a missing allowance inside the request transaction without blocking itself', async () => { + await ctx.prisma.employeeLeaveAllowance.deleteMany(); + const r = await create(); + expect(r.calculatedDays).toBe(5); + expect(await ctx.prisma.employeeLeaveAllowance.count()).toBe(1); + }); + + it('substitute decline → edit → resubmit restarts substitute approval and preserves the decline reason', async () => { + const r = await create('Vacation', other.id); + const declined = await ctx.http + .post(`/api/requests/${r.id}/substitute/decline`) + .set('Authorization', `Bearer ${other.token}`) + .send({ actorId: other.id, note: 'Bin selbst weg' }) + .expect(201); + expect(declined.body).toMatchObject({ + workflowState: 'Draft', + decisionNote: 'Bin selbst weg', + }); + const result = await submit(r.id) + .send(values(declined.body, { substituteId: other.id, halfDayEnd: true })) + .expect(201); + expect(result.body).toMatchObject({ + id: r.id, + workflowState: 'PendingSubstitute', + currentApproverId: null, + substituteId: other.id, + substituteAcceptedAt: null, + calculatedDays: 4.5, + }); + await ctx.http + .post(`/api/requests/${r.id}/substitute/accept`) + .set('Authorization', `Bearer ${other.token}`) + .send({ actorId: other.id }) + .expect(201); + const events = await ctx.prisma.requestEvent.findMany({ + where: { requestId: r.id }, + orderBy: { occurredAt: 'asc' }, + }); + expect(events.map((e) => e.kind)).toEqual([ + 'Submitted', + 'SubstituteDeclined', + 'Resubmitted', + 'SubstituteAccepted', + ]); + }); + + it('forbids other employees, manager and HR; ignores spoofed owner identity', async () => { + const r = await returned(); + for (const actor of [other, manager, hr]) { + await patch(r.id, actor.token) + .send(values(r, { employeeId: owner.id })) + .expect(403); + await submit(r.id, actor.token) + .send(values(r, { employeeId: owner.id })) + .expect(403); + } + await ctx.http.patch(`/api/requests/${r.id}`).send(values(r)).expect(401); + expect(await ctx.prisma.requestEvent.count()).toBe(2); + }); + + it.each([ + 'Submitted', + 'PendingSubstitute', + 'PendingManager', + 'PendingHr', + 'Approved', + 'Rejected', + 'Cancelled', + ] as const)( + 'refuses editing/resubmitting %s even with legacy status Submitted', + async (state) => { + const r = await returned(); + const current = await ctx.prisma.request.update({ + where: { id: r.id }, + data: { workflowState: state, status: 'Submitted' }, + }); + await patch(r.id) + .send(values({ updatedAt: current.updatedAt.toISOString() })) + .expect(409); + await submit(r.id) + .send(values({ updatedAt: current.updatedAt.toISOString() })) + .expect(409); + }, + ); + + it('rechecks vacation balance on both save and resubmit; does not spend the allowance twice', async () => { + const r = await returned(); + await seedLeaveAllowance(ctx.prisma, owner.id, YEAR, 0.5); + await patch(r.id).send(values(r)).expect(409); + await submit(r.id).send(values(r)).expect(409); + const result = await submit(r.id) + .send(values(r, { to: FROM, halfDayStart: true })) + .expect(201); + expect(result.body.calculatedDays).toBe(0.5); + }); + + it('excludes only its own overlap reservation, not another draft/active request', async () => { + const r = await returned(); + await ctx.prisma.request.create({ + data: { + employeeId: owner.id, + type: 'HomeOffice', + from: new Date(FROM), + to: new Date(FROM), + workflowState: 'Draft', + status: 'Submitted', + }, + }); + await patch(r.id).send(values(r)).expect(409); + await submit(r.id).send(values(r)).expect(409); + expect( + (await ctx.prisma.request.findUniqueOrThrow({ where: { id: r.id } })) + .workflowState, + ).toBe('Draft'); + }); + + it('revalidates dates, zero workdays, substitute and required version', async () => { + const r = await returned(); + for (const extra of [ + { from: TO, to: FROM }, + { to: '2027-01-01T00:00:00.000Z' }, + { from: '2026-05-09T00:00:00.000Z', to: '2026-05-10T00:00:00.000Z' }, + { substituteId: owner.id }, + { substituteId: '00000000-0000-0000-0000-000000000001' }, + { from: 'invalid' }, + { expectedUpdatedAt: undefined }, + ]) { + await submit(r.id).send(values(r, extra)).expect(400); + } + await ctx.prisma.employee.update({ + where: { id: other.id }, + data: { isActive: false }, + }); + await submit(r.id) + .send(values(r, { substituteId: other.id })) + .expect(400); + }); + + it('preserves SpecialLeave attachments and immutable type/owner', async () => { + const r = await returned('SpecialLeave'); + const attachment = await ctx.prisma.requestAttachment.create({ + data: { + requestId: r.id, + fileName: 'synthetic.pdf', + mimeType: 'application/pdf', + sizeBytes: 10, + storageKey: 'synthetic-only', + uploadedById: owner.id, + }, + }); + const result = await submit(r.id) + .send(values(r, { type: 'Vacation', employeeId: other.id })) + .expect(201); + expect(result.body).toMatchObject({ + id: r.id, + type: 'SpecialLeave', + employeeId: owner.id, + }); + expect( + await ctx.prisma.requestAttachment.findUnique({ + where: { id: attachment.id }, + }), + ).toMatchObject({ requestId: r.id, storageKey: 'synthetic-only' }); + }); + + it('revalidates TimeAdjustment overlap and off-hours approval', async () => { + const r = await returned('TimeAdjustment'); + const from = '2026-05-04T04:00:00.000Z'; // before the frame in UTC/Berlin + const to = '2026-05-04T08:00:00.000Z'; + await ctx.prisma.timeEntry.create({ + data: { + employeeId: owner.id, + clockIn: new Date(from), + clockOut: new Date(to), + source: 'Manual', + status: 'Approved', + }, + }); + await submit(r.id).send(values(r, { from, to })).expect(409); + await ctx.prisma.timeEntry.deleteMany(); + await submit(r.id) + .send(values(r, { from, to: from })) + .expect(400); + const result = await submit(r.id).send(values(r, { from, to })).expect(201); + expect(result.body.requiresApproval).toBe(true); + }); + + it('rejects stale edits and simultaneous resubmissions with no duplicate event', async () => { + const r = await returned(); + const changed = await patch(r.id) + .send(values(r, { reason: 'First tab' })) + .expect(200); + await patch(r.id) + .send(values(r, { reason: 'Stale tab' })) + .expect(409); + await submit(r.id).send(values(r)).expect(409); + const results = await Promise.all([ + submit(r.id).send(values(changed.body)), + submit(r.id).send(values(changed.body)), + ]); + expect(results.map((res) => res.status).sort()).toEqual([201, 409]); + expect( + await ctx.prisma.requestEvent.count({ + where: { requestId: r.id, kind: 'Resubmitted' }, + }), + ).toBe(1); + }); + + it('does not let an in-flight decision overwrite a returned and resubmitted version (ABA)', async () => { + const r = await create(); + const employees = ctx.app.get(EmployeesService); + const original = employees.getById.bind(employees); + let release!: () => void; + let arrived!: () => void; + const waiting = new Promise((resolve) => { + arrived = resolve; + }); + const barrier = new Promise((resolve) => { + release = resolve; + }); + let delayOnce = true; + const spy = jest + .spyOn(employees, 'getById') + .mockImplementation(async (id) => { + const employee = await original(id); + if (id === manager.id && delayOnce) { + delayOnce = false; + arrived(); + await barrier; + } + return employee; + }); + const oldDecision = ctx.http + .post(`/api/requests/${r.id}/manager-approve`) + .set('Authorization', `Bearer ${manager.token}`) + .send({ actorId: manager.id }) + .then((res) => res); + try { + await waiting; + const revised = await ctx.http + .post(`/api/requests/${r.id}/return`) + .set('Authorization', `Bearer ${hr.token}`) + .send({ actorId: hr.id, note: 'Neue Angaben nötig' }) + .expect(201); + await submit(r.id).send(values(revised.body)).expect(201); + release(); + expect((await oldDecision).status).toBe(409); + expect( + (await ctx.prisma.request.findUniqueOrThrow({ where: { id: r.id } })) + .workflowState, + ).toBe('PendingManager'); + expect( + await ctx.prisma.requestEvent.count({ + where: { requestId: r.id, kind: 'ManagerApproved' }, + }), + ).toBe(0); + } finally { + release(); + spy.mockRestore(); + } + }); + + it('serializes competing drafts so parallel submissions cannot overspend vacation', async () => { + const a = await returned(); + const b = await ctx.prisma.request.create({ + data: { + employeeId: owner.id, + type: 'Vacation', + from: new Date('2026-05-11'), + to: new Date('2026-05-15'), + workflowState: 'Draft', + calculatedDays: 5, + }, + }); + const results = await Promise.all([ + submit(a.id).send(values(a)), + submit(b.id).send( + values( + { updatedAt: b.updatedAt.toISOString() }, + { from: b.from.toISOString(), to: b.to.toISOString() }, + ), + ), + ]); + expect(results.map((res) => res.status).sort()).toEqual([201, 409]); + expect( + await ctx.prisma.request.count({ + where: { workflowState: 'PendingManager' }, + }), + ).toBe(1); + }); +}); diff --git a/apps/api/openapi.json b/apps/api/openapi.json index 1a60945..960390a 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -3394,7 +3394,17 @@ ], "responses": { "200": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/RequestDto" + } + } + } + } } }, "security": [ @@ -3419,7 +3429,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3445,7 +3462,55 @@ ], "responses": { "200": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } + } + }, + "security": [ + { + "bearer": [] + } + ], + "tags": ["requests"] + }, + "patch": { + "operationId": "RequestsController_updateDraft", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateDraftRequestDto" + } + } + } + }, + "responses": { + "200": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3498,7 +3563,57 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } + } + }, + "security": [ + { + "bearer": [] + } + ], + "tags": ["requests"] + } + }, + "/api/requests/{id}/resubmit": { + "post": { + "operationId": "RequestsController_resubmit", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UpdateDraftRequestDto" + } + } + } + }, + "responses": { + "201": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3534,7 +3649,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3570,7 +3692,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3606,7 +3735,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3642,7 +3778,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3678,7 +3821,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3714,7 +3864,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3750,7 +3907,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3786,7 +3950,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3822,7 +3993,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -3858,7 +4036,14 @@ }, "responses": { "201": { - "description": "" + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RequestDto" + } + } + } } }, "security": [ @@ -7656,6 +7841,128 @@ }, "required": ["baseDays", "carryOverDays", "adjustmentDays"] }, + "RequestDto": { + "type": "object", + "properties": { + "id": { + "type": "string", + "format": "uuid" + }, + "employeeId": { + "type": "string", + "format": "uuid" + }, + "employeeName": { + "type": "string" + }, + "updatedAt": { + "type": "string", + "format": "date-time" + }, + "type": { + "type": "string", + "enum": ["Vacation", "HomeOffice", "SpecialLeave", "TimeAdjustment"] + }, + "status": { + "type": "string" + }, + "workflowState": { + "type": "string" + }, + "from": { + "type": "string", + "format": "date-time" + }, + "to": { + "type": "string", + "format": "date-time" + }, + "reason": { + "type": "string", + "nullable": true + }, + "requiresApproval": { + "type": "boolean" + }, + "approverId": { + "type": "string", + "nullable": true, + "format": "uuid" + }, + "currentApproverId": { + "type": "string", + "nullable": true, + "format": "uuid" + }, + "substituteId": { + "type": "string", + "nullable": true, + "format": "uuid" + }, + "substituteAcceptedAt": { + "type": "string", + "nullable": true, + "format": "date-time" + }, + "hrConfirmedAt": { + "type": "string", + "nullable": true, + "format": "date-time" + }, + "cancelledAt": { + "type": "string", + "nullable": true, + "format": "date-time" + }, + "calculatedDays": { + "type": "number" + }, + "halfDayStart": { + "type": "boolean" + }, + "halfDayEnd": { + "type": "boolean" + }, + "decidedAt": { + "type": "string", + "nullable": true, + "format": "date-time" + }, + "decisionNote": { + "type": "string", + "nullable": true + }, + "createdAt": { + "type": "string", + "format": "date-time" + } + }, + "required": [ + "id", + "employeeId", + "employeeName", + "updatedAt", + "type", + "status", + "workflowState", + "from", + "to", + "reason", + "requiresApproval", + "approverId", + "currentApproverId", + "substituteId", + "substituteAcceptedAt", + "hrConfirmedAt", + "cancelledAt", + "calculatedDays", + "halfDayStart", + "halfDayEnd", + "decidedAt", + "decisionNote", + "createdAt" + ] + }, "CreateRequestDto": { "type": "object", "properties": { @@ -7678,7 +7985,7 @@ "example": "2026-08-07T00:00:00.000Z" }, "reason": { - "type": "object", + "type": "string", "nullable": true, "maxLength": 2000 } @@ -7701,12 +8008,12 @@ "format": "date-time" }, "substituteId": { - "type": "object", + "type": "string", "nullable": true, "format": "uuid" }, "reason": { - "type": "object", + "type": "string", "nullable": true, "maxLength": 2000 }, @@ -7723,6 +8030,45 @@ }, "required": ["employeeId", "from", "to"] }, + "UpdateDraftRequestDto": { + "type": "object", + "properties": { + "from": { + "type": "string", + "format": "date-time" + }, + "to": { + "type": "string", + "format": "date-time" + }, + "substituteId": { + "type": "string", + "nullable": true, + "format": "uuid" + }, + "reason": { + "type": "string", + "nullable": true, + "maxLength": 2000 + }, + "halfDayStart": { + "type": "boolean", + "default": false, + "description": "Take the first day as a half-day" + }, + "halfDayEnd": { + "type": "boolean", + "default": false, + "description": "Take the last day as a half-day" + }, + "expectedUpdatedAt": { + "type": "string", + "format": "date-time", + "description": "updatedAt from the version being edited; stale writes return 409" + } + }, + "required": ["from", "to", "expectedUpdatedAt"] + }, "TransitionDto": { "type": "object", "properties": { @@ -7731,7 +8077,7 @@ "format": "uuid" }, "note": { - "type": "object", + "type": "string", "nullable": true, "maxLength": 2000 } @@ -7746,7 +8092,7 @@ "format": "uuid" }, "note": { - "type": "object", + "type": "string", "nullable": true, "maxLength": 2000 }, @@ -7786,7 +8132,7 @@ } }, "note": { - "type": "object", + "type": "string", "nullable": true, "maxLength": 2000 }, diff --git a/apps/api/src/app/accounts/vacation-balance.service.ts b/apps/api/src/app/accounts/vacation-balance.service.ts index 61bc2b7..d50b4fb 100644 --- a/apps/api/src/app/accounts/vacation-balance.service.ts +++ b/apps/api/src/app/accounts/vacation-balance.service.ts @@ -1,3 +1,4 @@ +import type { Prisma } from '@prisma/client'; import { Injectable } from '@nestjs/common'; import { calculateWorkingDays } from 'shared'; import { LeaveAllowancesService } from '../leave-allowances/leave-allowances.service'; @@ -20,16 +21,22 @@ export class VacationBalanceService { private readonly schedules: WorkSchedulesService, ) {} - async compute(employeeId: string, year: number): Promise { - const allowance = await this.allowances.getOrDefault(employeeId, year); + async compute( + employeeId: string, + year: number, + excludeRequestId?: string, + db: Prisma.TransactionClient = this.prisma, + ): Promise { + const allowance = await this.allowances.getOrDefault(employeeId, year, db); const schedule = await this.schedules.resolveForEmployee(employeeId); const yearStart = new Date(Date.UTC(year, 0, 1)); const yearEnd = new Date(Date.UTC(year, 11, 31, 23, 59, 59)); - const requests = await this.prisma.request.findMany({ + const requests = await db.request.findMany({ where: { employeeId, type: 'Vacation', + ...(excludeRequestId ? { id: { not: excludeRequestId } } : {}), from: { lte: yearEnd }, to: { gte: yearStart }, }, diff --git a/apps/api/src/app/leave-allowances/leave-allowances.service.ts b/apps/api/src/app/leave-allowances/leave-allowances.service.ts index 76c1f78..4c4046c 100644 --- a/apps/api/src/app/leave-allowances/leave-allowances.service.ts +++ b/apps/api/src/app/leave-allowances/leave-allowances.service.ts @@ -1,5 +1,5 @@ import { Injectable, NotFoundException } from '@nestjs/common'; -import type { EmployeeLeaveAllowance } from '@prisma/client'; +import type { EmployeeLeaveAllowance, Prisma } from '@prisma/client'; import type { JwtUser } from '../auth/jwt.strategy'; import { EmployeesService } from '../employees/employees.service'; import { PrismaService } from '../prisma/prisma.service'; @@ -29,13 +29,14 @@ export class LeaveAllowancesService { async getOrDefault( employeeId: string, year: number, + db: Prisma.TransactionClient = this.prisma, ): Promise { - const existing = await this.prisma.employeeLeaveAllowance.findUnique({ + const existing = await db.employeeLeaveAllowance.findUnique({ where: { employeeId_year: { employeeId, year } }, }); if (existing) return existing; - const employee = await this.assertEmployee(employeeId); - return this.prisma.employeeLeaveAllowance.create({ + const employee = await this.assertEmployee(employeeId, db); + return db.employeeLeaveAllowance.create({ data: { employeeId, year, @@ -113,8 +114,11 @@ export class LeaveAllowancesService { return { scanned: candidates.length, expired }; } - private async assertEmployee(employeeId: string) { - const employee = await this.prisma.employee.findUnique({ + private async assertEmployee( + employeeId: string, + db: Prisma.TransactionClient = this.prisma, + ) { + const employee = await db.employee.findUnique({ where: { id: employeeId }, }); if (!employee) diff --git a/apps/api/src/app/requests/requests.controller.ts b/apps/api/src/app/requests/requests.controller.ts index ab41a63..40c6910 100644 --- a/apps/api/src/app/requests/requests.controller.ts +++ b/apps/api/src/app/requests/requests.controller.ts @@ -3,12 +3,18 @@ import { Controller, Get, Param, + Patch, ParseUUIDPipe, Post, Query, UseGuards, } from '@nestjs/common'; -import { ApiBearerAuth, ApiTags } from '@nestjs/swagger'; +import { + ApiBearerAuth, + ApiCreatedResponse, + ApiOkResponse, + ApiTags, +} from '@nestjs/swagger'; import { CurrentUser } from '../auth/current-user.decorator'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import type { JwtUser } from '../auth/jwt.strategy'; @@ -20,9 +26,10 @@ import { CreateVacationDto, ManagerApproveDto, TransitionDto, + UpdateDraftRequestDto, TransitionWithRequiredNoteDto, type BulkResult, - type RequestDto, + RequestDto, type RequestEventDto, } from './requests.dto'; @@ -34,6 +41,7 @@ export class RequestsController { constructor(private readonly service: RequestsService) {} @Get() + @ApiOkResponse({ type: RequestDto, isArray: true }) list( @CurrentUser() user: JwtUser, @Query('employeeId') employeeId?: string, @@ -57,6 +65,7 @@ export class RequestsController { } @Get(':id') + @ApiOkResponse({ type: RequestDto }) get( @Param('id', new ParseUUIDPipe()) id: string, @CurrentUser() user: JwtUser, @@ -73,6 +82,7 @@ export class RequestsController { } @Post() + @ApiCreatedResponse({ type: RequestDto }) create( @Body() dto: CreateRequestDto, @CurrentUser() user: JwtUser, @@ -81,6 +91,7 @@ export class RequestsController { } @Post('vacation') + @ApiCreatedResponse({ type: RequestDto }) createVacation( @Body() dto: CreateVacationDto, @CurrentUser() user: JwtUser, @@ -88,9 +99,30 @@ export class RequestsController { return this.service.createVacation({ ...dto, employeeId: user.id }); } + @Patch(':id') + @ApiOkResponse({ type: RequestDto }) + updateDraft( + @Param('id', new ParseUUIDPipe()) id: string, + @Body() dto: UpdateDraftRequestDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.updateDraft(id, user.id, dto, false); + } + + @Post(':id/resubmit') + @ApiCreatedResponse({ type: RequestDto }) + resubmit( + @Param('id', new ParseUUIDPipe()) id: string, + @Body() dto: UpdateDraftRequestDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.service.updateDraft(id, user.id, dto, true); + } + // ----- Generic approve / reject (legacy convenience) ----- @Post(':id/approve') + @ApiCreatedResponse({ type: RequestDto }) approve( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, @@ -100,6 +132,7 @@ export class RequestsController { } @Post(':id/reject') + @ApiCreatedResponse({ type: RequestDto }) reject( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, @@ -111,6 +144,7 @@ export class RequestsController { // ----- Vacation workflow ----- @Post(':id/manager-approve') + @ApiCreatedResponse({ type: RequestDto }) managerApprove( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: ManagerApproveDto, @@ -125,6 +159,7 @@ export class RequestsController { } @Post(':id/manager-reject') + @ApiCreatedResponse({ type: RequestDto }) managerReject( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, @@ -134,6 +169,7 @@ export class RequestsController { } @Post(':id/hr-confirm') + @ApiCreatedResponse({ type: RequestDto }) hrConfirm( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, @@ -143,6 +179,7 @@ export class RequestsController { } @Post(':id/hr-reject') + @ApiCreatedResponse({ type: RequestDto }) hrReject( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionWithRequiredNoteDto, @@ -152,6 +189,7 @@ export class RequestsController { } @Post(':id/substitute/accept') + @ApiCreatedResponse({ type: RequestDto }) substituteAccept( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, @@ -161,6 +199,7 @@ export class RequestsController { } @Post(':id/substitute/decline') + @ApiCreatedResponse({ type: RequestDto }) substituteDecline( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionWithRequiredNoteDto, @@ -170,6 +209,7 @@ export class RequestsController { } @Post(':id/return') + @ApiCreatedResponse({ type: RequestDto }) returnForRevision( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionWithRequiredNoteDto, @@ -179,6 +219,7 @@ export class RequestsController { } @Post(':id/cancel') + @ApiCreatedResponse({ type: RequestDto }) cancel( @Param('id', new ParseUUIDPipe()) id: string, @Body() body: TransitionDto, diff --git a/apps/api/src/app/requests/requests.dto.ts b/apps/api/src/app/requests/requests.dto.ts index 40e78f5..546ff70 100644 --- a/apps/api/src/app/requests/requests.dto.ts +++ b/apps/api/src/app/requests/requests.dto.ts @@ -1,4 +1,4 @@ -import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { ApiProperty, ApiPropertyOptional, OmitType } from '@nestjs/swagger'; import { ArrayNotEmpty, IsArray, @@ -12,7 +12,12 @@ import { } from 'class-validator'; import type { Request } from '@prisma/client'; -const REQUEST_TYPES = ['Vacation', 'HomeOffice', 'SpecialLeave', 'TimeAdjustment'] as const; +const REQUEST_TYPES = [ + 'Vacation', + 'HomeOffice', + 'SpecialLeave', + 'TimeAdjustment', +] as const; export class CreateRequestDto { @ApiProperty({ format: 'uuid' }) @@ -31,7 +36,7 @@ export class CreateRequestDto { @IsISO8601() to!: string; - @ApiPropertyOptional({ nullable: true, maxLength: 2000 }) + @ApiPropertyOptional({ type: String, nullable: true, maxLength: 2000 }) @IsOptional() @IsString() @MaxLength(2000) @@ -51,34 +56,53 @@ export class CreateVacationDto { @IsISO8601() to!: string; - @ApiPropertyOptional({ nullable: true, format: 'uuid' }) + @ApiPropertyOptional({ type: String, nullable: true, format: 'uuid' }) @IsOptional() @IsUUID() substituteId?: string | null; - @ApiPropertyOptional({ nullable: true, maxLength: 2000 }) + @ApiPropertyOptional({ type: String, nullable: true, maxLength: 2000 }) @IsOptional() @IsString() @MaxLength(2000) reason?: string | null; - @ApiPropertyOptional({ default: false, description: 'Take the first day as a half-day' }) + @ApiPropertyOptional({ + default: false, + description: 'Take the first day as a half-day', + }) @IsOptional() @IsBoolean() halfDayStart?: boolean; - @ApiPropertyOptional({ default: false, description: 'Take the last day as a half-day' }) + @ApiPropertyOptional({ + default: false, + description: 'Take the last day as a half-day', + }) @IsOptional() @IsBoolean() halfDayEnd?: boolean; } +/** Complete editable values. Request type and owner are immutable. */ +export class UpdateDraftRequestDto extends OmitType(CreateVacationDto, [ + 'employeeId', +] as const) { + @ApiProperty({ + format: 'date-time', + description: + 'updatedAt from the version being edited; stale writes return 409', + }) + @IsISO8601() + expectedUpdatedAt!: string; +} + export class TransitionDto { @ApiProperty({ format: 'uuid' }) @IsUUID() actorId!: string; - @ApiPropertyOptional({ nullable: true, maxLength: 2000 }) + @ApiPropertyOptional({ type: String, nullable: true, maxLength: 2000 }) @IsOptional() @IsString() @MaxLength(2000) @@ -114,7 +138,7 @@ export class BulkApproveDto { @IsUUID('all', { each: true }) ids!: string[]; - @ApiPropertyOptional({ nullable: true, maxLength: 2000 }) + @ApiPropertyOptional({ type: String, nullable: true, maxLength: 2000 }) @IsOptional() @IsString() @MaxLength(2000) @@ -151,34 +175,63 @@ export interface BulkResult { error?: string; } -export interface RequestDto { - id: string; - employeeId: string; - type: string; - status: string; - workflowState: string; - from: string; - to: string; - reason: string | null; - requiresApproval: boolean; - approverId: string | null; - currentApproverId: string | null; - substituteId: string | null; - substituteAcceptedAt: string | null; - hrConfirmedAt: string | null; - cancelledAt: string | null; - calculatedDays: number; - halfDayStart: boolean; - halfDayEnd: boolean; - decidedAt: string | null; - decisionNote: string | null; - createdAt: string; +export class RequestDto { + @ApiProperty({ format: 'uuid' }) + id!: string; + @ApiProperty({ format: 'uuid' }) + employeeId!: string; + @ApiProperty() + employeeName!: string; + @ApiProperty({ format: 'date-time' }) + updatedAt!: string; + @ApiProperty({ enum: REQUEST_TYPES }) + type!: string; + @ApiProperty() + status!: string; + @ApiProperty() + workflowState!: string; + @ApiProperty({ format: 'date-time' }) + from!: string; + @ApiProperty({ format: 'date-time' }) + to!: string; + @ApiProperty({ type: String, nullable: true }) + reason!: string | null; + @ApiProperty() + requiresApproval!: boolean; + @ApiProperty({ type: String, nullable: true, format: 'uuid' }) + approverId!: string | null; + @ApiProperty({ type: String, nullable: true, format: 'uuid' }) + currentApproverId!: string | null; + @ApiProperty({ type: String, nullable: true, format: 'uuid' }) + substituteId!: string | null; + @ApiProperty({ type: String, nullable: true, format: 'date-time' }) + substituteAcceptedAt!: string | null; + @ApiProperty({ type: String, nullable: true, format: 'date-time' }) + hrConfirmedAt!: string | null; + @ApiProperty({ type: String, nullable: true, format: 'date-time' }) + cancelledAt!: string | null; + @ApiProperty() + calculatedDays!: number; + @ApiProperty() + halfDayStart!: boolean; + @ApiProperty() + halfDayEnd!: boolean; + @ApiProperty({ type: String, nullable: true, format: 'date-time' }) + decidedAt!: string | null; + @ApiProperty({ type: String, nullable: true }) + decisionNote!: string | null; + @ApiProperty({ format: 'date-time' }) + createdAt!: string; } -export function toRequestDto(r: Request): RequestDto { +export function toRequestDto( + r: Request & { employee: { firstName: string; lastName: string } }, +): RequestDto { return { id: r.id, employeeId: r.employeeId, + employeeName: `${r.employee.firstName} ${r.employee.lastName}`, + updatedAt: r.updatedAt.toISOString(), type: r.type, status: r.status, workflowState: r.workflowState, @@ -189,7 +242,9 @@ export function toRequestDto(r: Request): RequestDto { approverId: r.approverId, currentApproverId: r.currentApproverId, substituteId: r.substituteId, - substituteAcceptedAt: r.substituteAcceptedAt ? r.substituteAcceptedAt.toISOString() : null, + substituteAcceptedAt: r.substituteAcceptedAt + ? r.substituteAcceptedAt.toISOString() + : null, hrConfirmedAt: r.hrConfirmedAt ? r.hrConfirmedAt.toISOString() : null, cancelledAt: r.cancelledAt ? r.cancelledAt.toISOString() : null, calculatedDays: Number(r.calculatedDays), diff --git a/apps/api/src/app/requests/requests.service.ts b/apps/api/src/app/requests/requests.service.ts index 95e562c..2a19d43 100644 --- a/apps/api/src/app/requests/requests.service.ts +++ b/apps/api/src/app/requests/requests.service.ts @@ -10,7 +10,6 @@ import type { Request, RequestEventKind, RequestType, - WorkflowState, } from '@prisma/client'; import { calculateVacationDays, @@ -32,6 +31,7 @@ import { type CreateVacationDto, type RequestDto, type RequestEventDto, + type UpdateDraftRequestDto, } from './requests.dto'; export interface ListRequestsFilter { @@ -43,6 +43,13 @@ export interface ListRequestsFilter { substituteId?: string; } +const employeeInclude = { + employee: { select: { firstName: true, lastName: true } }, +} as const; +type RequestWithEmployee = Prisma.RequestGetPayload<{ + include: typeof employeeInclude; +}>; + @Injectable() export class RequestsService { constructor( @@ -72,6 +79,7 @@ export class RequestsService { where: { AND: [where, await this.visibilityWhere(actorId)] }, orderBy: { createdAt: 'desc' }, take: 200, + include: employeeInclude, }); return rows.map(toRequestDto); } @@ -99,143 +107,234 @@ export class RequestsService { })); } - // ----- Generic create (HomeOffice, SpecialLeave, TimeAdjustment, Vacation-as-fallback) ----- - + // Creation and revision deliberately share validation. Serializing per employee + // keeps simultaneous resubmissions/creations from spending the same balance or + // passing the same overlap check. No request is deleted/recreated on revision. async create(dto: CreateRequestDto): Promise { - if (dto.type === 'Vacation') { - // Vacation has its own validated path with leave balance check. - return this.createVacation({ - employeeId: dto.employeeId, - from: dto.from, - to: dto.to, - substituteId: null, - reason: dto.reason ?? null, - }); - } - const employee = await this.employees.getById(dto.employeeId); - const from = new Date(dto.from); - const to = new Date(dto.to); - if (to.getTime() < from.getTime()) { - throw new BadRequestException('"to" must be on/after "from"'); - } - await this.assertNoActiveOverlap(employee.id, from, to); - const schedule = await this.schedules.resolveForEmployee(employee.id); - let requires = false; - if (dto.type === 'TimeAdjustment') { - if (to.getTime() <= from.getTime()) { - throw new BadRequestException( - 'TimeAdjustment "to" must be after "from"', - ); - } - await this.assertNoTimeEntryOverlap(employee.id, from, to); - requires = requiresSpecialApproval(from, to, schedule.frame); - } - const calculatedDays = calculateWorkingDays(from, to, { - workingDays: schedule.workingDays, - holidayProvider: schedule.holidayProvider, - }); + return this.createValidated(dto.type, dto); + } + + async createVacation(dto: CreateVacationDto): Promise { + return this.createValidated('Vacation', dto); + } + + private async lockEmployee( + tx: Prisma.TransactionClient, + employeeId: string, + ): Promise { + await tx.$queryRaw`SELECT "id" FROM "Employee" WHERE "id" = ${employeeId}::uuid FOR NO KEY UPDATE`; + } + + private async createValidated( + type: RequestType, + dto: CreateVacationDto, + ): Promise { const created = await this.prisma.$transaction(async (tx) => { + await this.lockEmployee(tx, dto.employeeId); + const values = await this.validateValues(type, dto, tx); const request = await tx.request.create({ data: { - employeeId: employee.id, - type: dto.type as RequestType, + ...values, + employeeId: dto.employeeId, + type, status: 'Submitted', - workflowState: 'PendingManager', - from, - to, - reason: dto.reason ?? null, - requiresApproval: requires, - calculatedDays, - currentApproverId: employee.managerId, + workflowState: values.substituteId + ? 'PendingSubstitute' + : 'PendingManager', }, + include: employeeInclude, }); await tx.requestEvent.create({ data: { requestId: request.id, kind: 'Submitted', - actorId: employee.id, + actorId: dto.employeeId, }, }); return request; }); - this.notifications.notifyTransitioned(created, 'Submitted', employee.id); + this.notifications.notifyTransitioned(created, 'Submitted', dto.employeeId); return toRequestDto(created); } - // ----- Vacation creation with balance check ----- + async updateDraft( + id: string, + actorId: string, + dto: UpdateDraftRequestDto, + resubmit: boolean, + ): Promise { + const updated = await this.prisma.$transaction(async (tx) => { + await this.lockEmployee(tx, actorId); + const request = await tx.request.findUnique({ where: { id } }); + if (!request) throw new NotFoundException(`Request ${id} not found`); + if (request.employeeId !== actorId) { + throw new ForbiddenException('Only the requester may edit or resubmit'); + } + if ( + request.workflowState !== 'Draft' || + request.updatedAt.toISOString() !== + new Date(dto.expectedUpdatedAt).toISOString() + ) { + throw new ConflictException( + 'Request changed or is no longer a draft; reload before editing', + ); + } + const values = await this.validateValues( + request.type, + { ...dto, employeeId: actorId }, + tx, + id, + ); + const data: Prisma.RequestUncheckedUpdateInput = { + ...values, + // A saved draft is not assigned until it is actually submitted. + currentApproverId: resubmit ? values.currentApproverId : null, + workflowState: resubmit + ? nextState('Draft', 'submit', { + hasSubstitute: !!values.substituteId, + }) + : 'Draft', + status: 'Submitted', // legacy display status; workflowState gates writes + approverId: null, + substituteAcceptedAt: null, + hrConfirmedAt: null, + cancelledAt: null, + decidedAt: null, + // Keep the revision reason visible until submission; history always keeps it. + decisionNote: resubmit ? null : request.decisionNote, + }; + const saved = await this.updateVersion(tx, request, data); + if (resubmit) { + await tx.requestEvent.create({ + data: { requestId: id, kind: 'Resubmitted', actorId }, + }); + } + return saved; + }); + if (resubmit) + this.notifications.notifyTransitioned(updated, 'Resubmitted', actorId); + return toRequestDto(updated); + } - async createVacation(dto: CreateVacationDto): Promise { + private async validateValues( + type: RequestType, + dto: CreateVacationDto, + tx: Prisma.TransactionClient, + excludeRequestId?: string, + ) { const employee = await this.employees.getById(dto.employeeId); const from = new Date(dto.from); const to = new Date(dto.to); - if (to.getTime() < from.getTime()) { - throw new BadRequestException('"to" must be on/after "from"'); + if ( + !Number.isFinite(from.getTime()) || + !Number.isFinite(to.getTime()) || + to < from + ) { + throw new BadRequestException('"to" must be on/after a valid "from"'); } - if (from.getUTCFullYear() !== to.getUTCFullYear()) { + const vacation = type === 'Vacation'; + if (vacation && from.getUTCFullYear() !== to.getUTCFullYear()) { throw new BadRequestException( 'Vacation must lie within a single calendar year — split into two requests', ); } - if (dto.substituteId && dto.substituteId === employee.id) { - throw new BadRequestException('Substitute must be a different employee'); + if (!vacation && (dto.substituteId || dto.halfDayStart || dto.halfDayEnd)) { + throw new BadRequestException( + 'Substitute and half-days are only supported for Vacation', + ); + } + const substituteId = vacation ? (dto.substituteId ?? null) : null; + if (substituteId) { + const substitute = await tx.employee.findUnique({ + where: { id: substituteId }, + }); + if (substituteId === employee.id || !substitute?.isActive) { + throw new BadRequestException( + 'Substitute must be a different active employee', + ); + } } const schedule = await this.schedules.resolveForEmployee(employee.id); - const halfDayStart = !!dto.halfDayStart; - const halfDayEnd = !!dto.halfDayEnd; - const calculatedDays = calculateVacationDays(from, to, { + const halfDayStart = vacation && !!dto.halfDayStart; + const halfDayEnd = vacation && !!dto.halfDayEnd; + const options = { workingDays: schedule.workingDays, holidayProvider: schedule.holidayProvider, halfDayStart, halfDayEnd, - }); - if (calculatedDays <= 0) { + }; + const calculatedDays = vacation + ? calculateVacationDays(from, to, options) + : calculateWorkingDays(from, to, options); + if (vacation && calculatedDays <= 0) throw new BadRequestException('Vacation range covers no working days'); - } - await this.assertNoActiveOverlap(employee.id, from, to); - - const balance = await this.vacationBalance.compute( + await this.assertNoActiveOverlap( employee.id, - from.getUTCFullYear(), + from, + to, + tx, + excludeRequestId, ); - if (balance.remainingDays < calculatedDays) { - throw new ConflictException( - `Not enough vacation days remaining (${balance.remainingDays} < ${calculatedDays})`, + let requiresApproval = false; + if (type === 'TimeAdjustment') { + if (to <= from) + throw new BadRequestException( + 'TimeAdjustment "to" must be after "from"', + ); + await this.assertNoTimeEntryOverlap(employee.id, from, to, tx); + requiresApproval = requiresSpecialApproval(from, to, schedule.frame); + } + if (vacation) { + const balance = await this.vacationBalance.compute( + employee.id, + from.getUTCFullYear(), + excludeRequestId, + tx, ); + if (balance.remainingDays < calculatedDays) { + throw new ConflictException( + `Not enough vacation days remaining (${balance.remainingDays} < ${calculatedDays})`, + ); + } } + return { + from, + to, + reason: dto.reason ?? null, + calculatedDays, + halfDayStart, + halfDayEnd, + substituteId, + requiresApproval, + currentApproverId: substituteId ? null : employee.managerId, + }; + } - const hasSubstitute = !!dto.substituteId; - const initialState: WorkflowState = hasSubstitute - ? 'PendingSubstitute' - : 'PendingManager'; - - const created = await this.prisma.$transaction(async (tx) => { - const request = await tx.request.create({ - data: { - employeeId: employee.id, - type: 'Vacation', - status: 'Submitted', - workflowState: initialState, - from, - to, - reason: dto.reason ?? null, - calculatedDays, - halfDayStart, - halfDayEnd, - substituteId: dto.substituteId ?? null, - currentApproverId: hasSubstitute ? null : employee.managerId, - }, - }); - await tx.requestEvent.create({ - data: { - requestId: request.id, - kind: 'Submitted', - actorId: employee.id, - }, - }); - return request; + /** Compare-and-swap protects transitions too, including Draft → pending → Draft ABA races. */ + private async updateVersion( + tx: Prisma.TransactionClient, + request: Request, + data: Prisma.RequestUncheckedUpdateInput, + ): Promise { + const changed = await tx.request.updateMany({ + where: { + id: request.id, + workflowState: request.workflowState, + updatedAt: request.updatedAt, + }, + data: { + ...data, + updatedAt: new Date( + Math.max(Date.now(), request.updatedAt.getTime() + 1), + ), + }, + }); + if (changed.count !== 1) + throw new ConflictException('Request changed; reload before continuing'); + return tx.request.findUniqueOrThrow({ + where: { id: request.id }, + include: employeeInclude, }); - this.notifications.notifyTransitioned(created, 'Submitted', employee.id); - return toRequestDto(created); } // ----- Generic approve / reject (legacy path for non-Vacation) ----- @@ -263,23 +362,21 @@ export class RequestsService { ); } await this.assertApprover(request, actorId); - const alreadyApproved = request.workflowState === 'Approved'; + if (request.workflowState !== 'PendingManager') + throw new ConflictException('Request is not pending manager approval'); const updated = await this.prisma.$transaction(async (tx) => { - const updated = await tx.request.update({ - where: { id: request.id }, - data: { - status: 'Approved', - workflowState: 'Approved', - approverId: actorId, - currentApproverId: null, - decidedAt: new Date(), - decisionNote: note, - }, + const updated = await this.updateVersion(tx, request, { + status: 'Approved', + workflowState: 'Approved', + approverId: actorId, + currentApproverId: null, + decidedAt: new Date(), + decisionNote: note, }); await tx.requestEvent.create({ data: { requestId: id, kind: 'ManagerApproved', actorId, note }, }); - if (request.type === 'TimeAdjustment' && !alreadyApproved) { + if (request.type === 'TimeAdjustment') { await this.materializeTimeAdjustment(tx, request); } return updated; @@ -299,17 +396,16 @@ export class RequestsService { return this.transitionVacation(request, 'manager_reject', actorId, note); } await this.assertApprover(request, actorId); + if (request.workflowState !== 'PendingManager') + throw new ConflictException('Request is not pending manager approval'); const updated = await this.prisma.$transaction(async (tx) => { - const updated = await tx.request.update({ - where: { id: request.id }, - data: { - status: 'Rejected', - workflowState: 'Rejected', - approverId: actorId, - currentApproverId: null, - decidedAt: new Date(), - decisionNote: note, - }, + const updated = await this.updateVersion(tx, request, { + status: 'Rejected', + workflowState: 'Rejected', + approverId: actorId, + currentApproverId: null, + decidedAt: new Date(), + decisionNote: note, }); await tx.requestEvent.create({ data: { requestId: id, kind: 'ManagerRejected', actorId, note }, @@ -578,7 +674,7 @@ export class RequestsService { }); const status = deriveStatus(target); - const data: Prisma.RequestUpdateInput = { + const data: Prisma.RequestUncheckedUpdateInput = { workflowState: target, status, }; @@ -588,48 +684,47 @@ export class RequestsService { // Side effects per transition. if (event === 'submit') { - // No-op here — submit is done at create time. + // Creation and resubmission have their own validated paths. } if (event === 'substitute_accept') { data.substituteAcceptedAt = new Date(); // Move approver to manager. const employee = await this.employees.getById(employeeId); - data.currentApprover = employee.managerId - ? { connect: { id: employee.managerId } } - : { disconnect: true }; + data.currentApproverId = employee.managerId; } if (event === 'substitute_decline') { - data.currentApprover = { disconnect: true }; + data.decisionNote = note; + data.currentApproverId = null; } if (event === 'manager_approve' || event === 'manager_approve_with_hr') { - data.approver = { connect: { id: actorId } }; + data.approverId = actorId; data.decidedAt = event === 'manager_approve' ? new Date() : null; data.decisionNote = event === 'manager_approve' ? note : null; if (event === 'manager_approve_with_hr') { - data.currentApprover = { disconnect: true }; + data.currentApproverId = null; } else { - data.currentApprover = { disconnect: true }; + data.currentApproverId = null; } } if (event === 'manager_reject' || event === 'manager_return') { - data.approver = { connect: { id: actorId } }; + data.approverId = actorId; data.decidedAt = event === 'manager_reject' ? new Date() : null; data.decisionNote = note; - data.currentApprover = { disconnect: true }; + data.currentApproverId = null; } if (event === 'hr_confirm' || event === 'hr_reject') { data.hrConfirmedAt = event === 'hr_confirm' ? new Date() : null; data.decidedAt = new Date(); data.decisionNote = note; - data.currentApprover = { disconnect: true }; + data.currentApproverId = null; } if (event === 'cancel') { data.cancelledAt = new Date(); - data.currentApprover = { disconnect: true }; + data.currentApproverId = null; } const updated = await this.prisma.$transaction(async (tx) => { - const u = await tx.request.update({ where: { id: request.id }, data }); + const u = await this.updateVersion(tx, request, data); await tx.requestEvent.create({ data: { requestId: request.id, kind: eventKind, actorId, note }, }); @@ -672,8 +767,11 @@ export class RequestsService { }); } - private async assertRequest(id: string): Promise { - const request = await this.prisma.request.findUnique({ where: { id } }); + private async assertRequest(id: string): Promise { + const request = await this.prisma.request.findUnique({ + where: { id }, + include: employeeInclude, + }); if (!request) throw new NotFoundException(`Request ${id} not found`); return request; } @@ -737,11 +835,14 @@ export class RequestsService { employeeId: string, from: Date, to: Date, + db: Prisma.TransactionClient = this.prisma, + excludeRequestId?: string, ): Promise { - const overlap = await this.prisma.request.findFirst({ + const overlap = await db.request.findFirst({ where: { employeeId, status: { notIn: ['Rejected', 'Cancelled'] }, + ...(excludeRequestId ? { id: { not: excludeRequestId } } : {}), from: { lte: to }, to: { gte: from }, }, @@ -762,8 +863,9 @@ export class RequestsService { employeeId: string, from: Date, to: Date, + db: Prisma.TransactionClient = this.prisma, ): Promise { - const overlap = await this.prisma.timeEntry.findFirst({ + const overlap = await db.timeEntry.findFirst({ where: { employeeId, status: { not: 'Rejected' }, diff --git a/apps/web/src/api/client.ts b/apps/web/src/api/client.ts index 037c1a3..d47a9e0 100644 --- a/apps/web/src/api/client.ts +++ b/apps/web/src/api/client.ts @@ -367,6 +367,8 @@ export interface AccountDto { export interface RequestDto { id: string; employeeId: string; + employeeName: string; + updatedAt: string; type: RequestType; status: RequestStatus; workflowState: WorkflowState; @@ -546,6 +548,13 @@ export interface CreateVacationPayload { halfDayEnd?: boolean; } +export interface UpdateDraftRequestPayload extends Omit< + CreateVacationPayload, + 'employeeId' +> { + expectedUpdatedAt: string; +} + export interface UpsertLeaveAllowancePayload { baseDays: number; carryOverDays: number; @@ -1055,6 +1064,16 @@ export const api = { method: 'POST', body: JSON.stringify(payload), }), + updateDraftRequest: (id: string, payload: UpdateDraftRequestPayload) => + request(`/api/requests/${id}`, { + method: 'PATCH', + body: JSON.stringify(payload), + }), + resubmitRequest: (id: string, payload: UpdateDraftRequestPayload) => + request(`/api/requests/${id}/resubmit`, { + method: 'POST', + body: JSON.stringify(payload), + }), approveRequest: (id: string, actorId: string, note?: string) => request(`/api/requests/${id}/approve`, { method: 'POST', diff --git a/apps/web/src/api/generated.ts b/apps/web/src/api/generated.ts index 4f7538e..41d8884 100644 --- a/apps/web/src/api/generated.ts +++ b/apps/web/src/api/generated.ts @@ -1281,7 +1281,7 @@ export interface paths { delete?: never; options?: never; head?: never; - patch?: never; + patch: operations['RequestsController_updateDraft']; trace?: never; }; '/api/requests/{id}/events': { @@ -1316,6 +1316,22 @@ export interface paths { patch?: never; trace?: never; }; + '/api/requests/{id}/resubmit': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations['RequestsController_resubmit']; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; '/api/requests/{id}/approve': { parameters: { query?: never; @@ -2813,6 +2829,45 @@ export interface components { adjustmentDays: number; adjustmentReason?: Record | null; }; + RequestDto: { + /** Format: uuid */ + id: string; + /** Format: uuid */ + employeeId: string; + employeeName: string; + /** Format: date-time */ + updatedAt: string; + /** @enum {string} */ + type: 'Vacation' | 'HomeOffice' | 'SpecialLeave' | 'TimeAdjustment'; + status: string; + workflowState: string; + /** Format: date-time */ + from: string; + /** Format: date-time */ + to: string; + reason: string | null; + requiresApproval: boolean; + /** Format: uuid */ + approverId: string | null; + /** Format: uuid */ + currentApproverId: string | null; + /** Format: uuid */ + substituteId: string | null; + /** Format: date-time */ + substituteAcceptedAt: string | null; + /** Format: date-time */ + hrConfirmedAt: string | null; + /** Format: date-time */ + cancelledAt: string | null; + calculatedDays: number; + halfDayStart: boolean; + halfDayEnd: boolean; + /** Format: date-time */ + decidedAt: string | null; + decisionNote: string | null; + /** Format: date-time */ + createdAt: string; + }; CreateRequestDto: { /** Format: uuid */ employeeId: string; @@ -2828,7 +2883,7 @@ export interface components { * @example 2026-08-07T00:00:00.000Z */ to: string; - reason?: Record | null; + reason?: string | null; }; CreateVacationDto: { /** Format: uuid */ @@ -2838,8 +2893,8 @@ export interface components { /** Format: date-time */ to: string; /** Format: uuid */ - substituteId?: Record | null; - reason?: Record | null; + substituteId?: string | null; + reason?: string | null; /** * @description Take the first day as a half-day * @default false @@ -2851,15 +2906,39 @@ export interface components { */ halfDayEnd: boolean; }; + UpdateDraftRequestDto: { + /** Format: date-time */ + from: string; + /** Format: date-time */ + to: string; + /** Format: uuid */ + substituteId?: string | null; + reason?: string | null; + /** + * @description Take the first day as a half-day + * @default false + */ + halfDayStart: boolean; + /** + * @description Take the last day as a half-day + * @default false + */ + halfDayEnd: boolean; + /** + * Format: date-time + * @description updatedAt from the version being edited; stale writes return 409 + */ + expectedUpdatedAt: string; + }; TransitionDto: { /** Format: uuid */ actorId: string; - note?: Record | null; + note?: string | null; }; ManagerApproveDto: { /** Format: uuid */ actorId: string; - note?: Record | null; + note?: string | null; /** @default false */ requiresHrConfirmation: boolean; }; @@ -2872,7 +2951,7 @@ export interface components { /** Format: uuid */ actorId: string; ids: string[]; - note?: Record | null; + note?: string | null; /** @default false */ requiresHrConfirmation: boolean; }; @@ -5059,7 +5138,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto'][]; + }; }; }; }; @@ -5080,7 +5161,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5099,7 +5182,34 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; + }; + }; + }; + RequestsController_updateDraft: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody: { + content: { + 'application/json': components['schemas']['UpdateDraftRequestDto']; + }; + }; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5139,7 +5249,34 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; + }; + }; + }; + RequestsController_resubmit: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody: { + content: { + 'application/json': components['schemas']['UpdateDraftRequestDto']; + }; + }; + responses: { + 201: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5162,7 +5299,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5185,7 +5324,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5208,7 +5349,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5231,7 +5374,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5254,7 +5399,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5277,7 +5424,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5300,7 +5449,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5323,7 +5474,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5346,7 +5499,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; @@ -5369,7 +5524,9 @@ export interface operations { headers: { [name: string]: unknown; }; - content?: never; + content: { + 'application/json': components['schemas']['RequestDto']; + }; }; }; }; diff --git a/apps/web/src/app/i18n.tsx b/apps/web/src/app/i18n.tsx index bb83a16..eeea78d 100644 --- a/apps/web/src/app/i18n.tsx +++ b/apps/web/src/app/i18n.tsx @@ -347,6 +347,15 @@ const de: Catalog = { 'Für den ausgewählten Tag wurde bereits ein Tagesblock gebucht.', 'requests.title': 'Anträge', 'requests.description': 'Urlaub, Home-Office, Sonderurlaub, Zeitkorrekturen', + 'requests.edit': 'Antrag bearbeiten', + 'requests.resubmit': 'Erneut einreichen', + 'enum.Resubmitted': 'Erneut eingereicht', + 'requests.saveDraft': 'Entwurf speichern', + 'requests.employee': 'Mitarbeiter/in', + 'requests.revisionReason': 'Rückgabegrund', + 'requests.notDraft': + 'Dieser Antrag ist kein Entwurf mehr. Bitte Übersicht neu laden.', + 'requests.loadFailed': 'Antrag konnte nicht geladen werden.', 'requests.new': 'Neuer Antrag', 'requests.own': 'Eigene Anträge', 'requests.none': 'Noch keine Anträge.', @@ -1018,6 +1027,15 @@ const en: Catalog = { 'requests.title': 'Requests', 'requests.description': 'Vacation, home office, special leave, time adjustments', + 'requests.edit': 'Edit request', + 'requests.resubmit': 'Resubmit', + 'enum.Resubmitted': 'Resubmitted', + 'requests.saveDraft': 'Save draft', + 'requests.employee': 'Employee', + 'requests.revisionReason': 'Reason for revision', + 'requests.notDraft': + 'This request is no longer a draft. Please reload the list.', + 'requests.loadFailed': 'Could not load request.', 'requests.new': 'New request', 'requests.own': 'My requests', 'requests.none': 'No requests yet.', diff --git a/apps/web/src/routes/AdminRequestsPage.spec.tsx b/apps/web/src/routes/AdminRequestsPage.spec.tsx new file mode 100644 index 0000000..30274bf --- /dev/null +++ b/apps/web/src/routes/AdminRequestsPage.spec.tsx @@ -0,0 +1,52 @@ +import { describe, expect, it, vi } from 'vitest'; +import { fireEvent, screen } from '@testing-library/react'; +import { renderWithProviders } from '../test-utils'; +import { AdminRequestsPage } from './AdminRequestsPage'; + +const request = { + id: 'request-42', + employeeId: 'owner', + employeeName: 'Anna Example', + type: 'Vacation', + workflowState: 'PendingManager', + status: 'Submitted', + currentApproverId: 'manager', + from: '2026-05-04T00:00:00.000Z', + to: '2026-05-08T00:00:00.000Z', + calculatedDays: 5, +}; +vi.mock('../api/client', () => ({ + api: { + listRequests: vi + .fn() + .mockImplementation(async (filter) => + filter.currentApproverId ? [request] : [], + ), + getRequest: vi.fn().mockImplementation(async () => request), + getRequestEvents: vi + .fn() + .mockResolvedValue([ + { id: 'event', kind: 'Resubmitted', at: '2026-05-01T12:00:00.000Z' }, + ]), + }, +})); +vi.mock('../app/auth', () => ({ + useCurrentUser: () => ({ + id: 'manager', + firstName: 'Marc', + lastName: 'Example', + role: 'Manager', + }), +})); + +describe('request identity in approval context (#42)', () => { + it('shows the employee name in the inbox and retained audit trail', async () => { + renderWithProviders(); + expect(await screen.findByText(/Anna Example · Urlaub/)).toBeDefined(); + fireEvent.click(screen.getByRole('button', { name: 'Verlauf' })); + expect( + await screen.findByText(/Anna Example · Urlaub · 4.5.2026/), + ).toBeDefined(); + expect(await screen.findByText('Erneut eingereicht')).toBeDefined(); + }); +}); diff --git a/apps/web/src/routes/AdminRequestsPage.tsx b/apps/web/src/routes/AdminRequestsPage.tsx index d8fc2df..e7ef36f 100644 --- a/apps/web/src/routes/AdminRequestsPage.tsx +++ b/apps/web/src/routes/AdminRequestsPage.tsx @@ -502,7 +502,7 @@ function RequestRow({ )}

- {enumLabel(request.type)} ·{' '} + {request.employeeName} · {enumLabel(request.type)} ·{' '} {isTimeAdjustment ? `${formatDateTime(request.from)} – ${formatDateTime(request.to)}` : `${formatDate(request.from.slice(0, 10))} – ${formatDate(request.to.slice(0, 10))}`} @@ -624,7 +624,7 @@ function AuditTrail({ requestId }: { requestId: string }) { {t('requests.auditHistory')} {requestQuery.data - ? `${enumLabel(requestQuery.data.type)} · ${formatDate(requestQuery.data.type === 'TimeAdjustment' ? requestQuery.data.from : requestQuery.data.from.slice(0, 10))} – ${formatDate(requestQuery.data.type === 'TimeAdjustment' ? requestQuery.data.to : requestQuery.data.to.slice(0, 10))}` + ? `${requestQuery.data.employeeName} · ${enumLabel(requestQuery.data.type)} · ${formatDate(requestQuery.data.type === 'TimeAdjustment' ? requestQuery.data.from : requestQuery.data.from.slice(0, 10))} – ${formatDate(requestQuery.data.type === 'TimeAdjustment' ? requestQuery.data.to : requestQuery.data.to.slice(0, 10))}` : '…'} diff --git a/apps/web/src/routes/RequestsPage.spec.tsx b/apps/web/src/routes/RequestsPage.spec.tsx index 933cab4..6479619 100644 --- a/apps/web/src/routes/RequestsPage.spec.tsx +++ b/apps/web/src/routes/RequestsPage.spec.tsx @@ -3,13 +3,22 @@ import { fireEvent, screen, waitFor, within } from '@testing-library/react'; import { renderWithProviders } from '../test-utils'; import { RequestsPage } from './RequestsPage'; +const listRequestsMock = vi.fn(); +const getRequestMock = vi.fn(); +const getRequestEventsMock = vi.fn(); +const updateDraftRequestMock = vi.fn(); +const resubmitRequestMock = vi.fn(); const employeeDirectoryMock = vi.fn(); const employeesMock = vi.fn(); const createVacationRequestMock = vi.fn(); vi.mock('../api/client', () => ({ api: { - listRequests: vi.fn().mockResolvedValue([]), + listRequests: (...args: unknown[]) => listRequestsMock(...args), + getRequest: (...args: unknown[]) => getRequestMock(...args), + getRequestEvents: (...args: unknown[]) => getRequestEventsMock(...args), + updateDraftRequest: (...args: unknown[]) => updateDraftRequestMock(...args), + resubmitRequest: (...args: unknown[]) => resubmitRequestMock(...args), employeeDirectory: (...args: unknown[]) => employeeDirectoryMock(...args), employees: (...args: unknown[]) => employeesMock(...args), vacationBalance: vi.fn().mockResolvedValue({ @@ -34,6 +43,11 @@ vi.mock('../app/auth', () => ({ describe('vacation substitute selection', () => { beforeEach(() => { + listRequestsMock.mockReset().mockResolvedValue([]); + getRequestMock.mockReset(); + getRequestEventsMock.mockReset().mockResolvedValue([]); + updateDraftRequestMock.mockReset().mockResolvedValue({}); + resubmitRequestMock.mockReset().mockResolvedValue({}); employeesMock.mockClear(); employeeDirectoryMock.mockClear().mockResolvedValue([ { id: 'requester', firstName: 'Anna', lastName: 'Example' }, @@ -72,4 +86,126 @@ describe('vacation substitute selection', () => { ); }); }); + const draft = { + id: 'same-request', + employeeId: 'requester', + employeeName: 'Anna Example', + type: 'Vacation', + workflowState: 'Draft', + status: 'Submitted', + from: '2026-05-04T00:00:00.000Z', + to: '2026-05-08T00:00:00.000Z', + reason: 'Original reason', + decisionNote: 'Bitte kürzen', + substituteId: 'substitute', + halfDayStart: true, + halfDayEnd: false, + calculatedDays: 4.5, + updatedAt: '2026-05-01T12:00:00.001Z', + }; + + it('opens a draft by keyboard-accessible action with current values/name/reason and resubmits the same ID', async () => { + listRequestsMock.mockResolvedValue([draft]); + getRequestMock.mockResolvedValue(draft); + renderWithProviders(); + fireEvent.click( + await screen.findByRole('button', { + name: 'Entwurf · Antrag bearbeiten', + }), + ); + expect( + await screen.findByText('Rückgabegrund: Bitte kürzen'), + ).toBeDefined(); + expect(screen.getByText('Mitarbeiter/in: Anna Example')).toBeDefined(); + expect((screen.getByLabelText('Von') as HTMLInputElement).value).toBe( + '2026-05-04', + ); + expect((screen.getByLabelText('Bis') as HTMLInputElement).value).toBe( + '2026-05-08', + ); + expect( + ( + (await screen.findByLabelText( + 'Vertretung (optional)', + )) as HTMLSelectElement + ).value, + ).toBe('substitute'); + expect( + (screen.getByLabelText('Erster Tag halb') as HTMLInputElement).checked, + ).toBe(true); + fireEvent.change(screen.getByLabelText('Bis'), { + target: { value: '2026-05-07' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Erneut einreichen' })); + await waitFor(() => + expect(resubmitRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ + expectedUpdatedAt: draft.updatedAt, + reason: 'Original reason', + to: '2026-05-07T00:00:00.000Z', + substituteId: 'substitute', + halfDayStart: true, + }), + ), + ); + expect(createVacationRequestMock).not.toHaveBeenCalled(); + }); + + it('keeps stale/failed edits visible and never creates a replacement request', async () => { + listRequestsMock.mockResolvedValue([draft]); + getRequestMock.mockResolvedValue(draft); + resubmitRequestMock.mockRejectedValue( + new Error('Request changed; reload before continuing'), + ); + renderWithProviders(); + fireEvent.click( + await screen.findByRole('button', { + name: 'Entwurf · Antrag bearbeiten', + }), + ); + await screen.findByText('Rückgabegrund: Bitte kürzen'); + fireEvent.click(screen.getByRole('button', { name: 'Erneut einreichen' })); + expect( + await screen.findByText('Request changed; reload before continuing'), + ).toBeDefined(); + expect(screen.getByRole('dialog')).toBeDefined(); + expect(createVacationRequestMock).not.toHaveBeenCalled(); + }); + + it('can save a draft without submission and reads legacy decline reasons from the retained history', async () => { + listRequestsMock.mockResolvedValue([draft]); + getRequestMock.mockResolvedValue({ ...draft, decisionNote: null }); + getRequestEventsMock.mockResolvedValue([ + { kind: 'SubstituteDeclined', note: 'Bin selbst weg' }, + ]); + renderWithProviders(); + fireEvent.click( + await screen.findByRole('button', { + name: 'Entwurf · Antrag bearbeiten', + }), + ); + expect( + await screen.findByText('Rückgabegrund: Bin selbst weg'), + ).toBeDefined(); + fireEvent.click(screen.getByRole('button', { name: 'Entwurf speichern' })); + await waitFor(() => + expect(updateDraftRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ expectedUpdatedAt: draft.updatedAt }), + ), + ); + expect(resubmitRequestMock).not.toHaveBeenCalled(); + }); + + it('does not offer editing for pending requests', async () => { + listRequestsMock.mockResolvedValue([ + { ...draft, workflowState: 'PendingManager' }, + ]); + renderWithProviders(); + await screen.findByText(/Anna Example · Urlaub/); + expect( + screen.queryByRole('button', { name: /Antrag bearbeiten/ }), + ).toBeNull(); + }); }); diff --git a/apps/web/src/routes/RequestsPage.tsx b/apps/web/src/routes/RequestsPage.tsx index ce0534b..f893024 100644 --- a/apps/web/src/routes/RequestsPage.tsx +++ b/apps/web/src/routes/RequestsPage.tsx @@ -15,7 +15,7 @@ import { } from '@/components/ui/dialog'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; -import { api, type RequestType } from '../api/client'; +import { api, type RequestDto, type RequestType } from '../api/client'; import { useCurrentUser } from '../app/auth'; import { useI18n } from '../app/i18n'; @@ -55,6 +55,7 @@ export function RequestsPage() { const employeeId = user.id; const qc = useQueryClient(); const [dialogOpen, setDialogOpen] = useState(false); + const [editingId, setEditingId] = useState(null); const requests = useQuery({ queryKey: ['requests', { employeeId }], @@ -76,17 +77,30 @@ export function RequestsPage() { - + { setDialogOpen(false); qc.invalidateQueries({ queryKey: ['requests'] }); + qc.invalidateQueries({ queryKey: ['vacation-balance'] }); }} />

+ {editingId && ( + { + setEditingId(null); + qc.invalidateQueries({ queryKey: ['requests'] }); + qc.invalidateQueries({ queryKey: ['request'] }); + qc.invalidateQueries({ queryKey: ['request-events'] }); + qc.invalidateQueries({ queryKey: ['vacation-balance'] }); + }} + /> + )} {t('requests.own')} @@ -101,6 +115,7 @@ export function RequestsPage() { >

+ {r.employeeName || `${user.firstName} ${user.lastName}`} ·{' '} {enumLabel(r.type)} ·{' '} {formatRange( r.type as RequestType, @@ -122,9 +137,19 @@ export function RequestsPage() { {t('requests.specialApproval')} )} - - {enumLabel(r.workflowState)} - + {r.workflowState === 'Draft' ? ( + + ) : ( + + {enumLabel(r.workflowState)} + + )}

{r.type === 'SpecialLeave' && ( @@ -143,31 +168,74 @@ export function RequestsPage() { ); } +function EditRequestDialog({ + requestId, + onClose, +}: { + requestId: string; + onClose: () => void; +}) { + const { t } = useI18n(); + const request = useQuery({ + queryKey: ['request', requestId], + queryFn: () => api.getRequest(requestId), + staleTime: 0, + refetchOnWindowFocus: false, + }); + return ( + !open && onClose()}> + + {request.data && !request.isFetching ? ( + + ) : ( + + {t('requests.edit')} + + {request.isError ? t('requests.loadFailed') : t('common.loading')} + + + )} + + + ); +} + interface NewRequestFormProps { onClose: () => void; + request?: RequestDto; } -function isoDateTimeLocalNow(): string { - const d = new Date(); +function isoDateTimeLocalNow(iso?: string): string { + const d = iso ? new Date(iso) : new Date(); // datetime-local needs YYYY-MM-DDTHH:mm in *local* time const pad = (n: number) => String(n).padStart(2, '0'); return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T${pad(d.getHours())}:${pad(d.getMinutes())}`; } -function NewRequestForm({ onClose }: NewRequestFormProps) { +function NewRequestForm({ onClose, request }: NewRequestFormProps) { const user = useCurrentUser(); const { t, enumLabel } = useI18n(); const employeeId = user.id; const today = isoDateOnly(new Date()); - const [type, setType] = useState('Vacation'); - const [from, setFrom] = useState(today); - const [to, setTo] = useState(today); - const [fromDt, setFromDt] = useState(isoDateTimeLocalNow); - const [toDt, setToDt] = useState(isoDateTimeLocalNow); - const [reason, setReason] = useState(''); - const [substituteId, setSubstituteId] = useState(''); - const [halfDayStart, setHalfDayStart] = useState(false); - const [halfDayEnd, setHalfDayEnd] = useState(false); + const [type, setType] = useState(request?.type ?? 'Vacation'); + const [from, setFrom] = useState(request?.from.slice(0, 10) ?? today); + const [to, setTo] = useState(request?.to.slice(0, 10) ?? today); + const [fromDt, setFromDt] = useState(() => + isoDateTimeLocalNow(request?.from), + ); + const [toDt, setToDt] = useState(() => isoDateTimeLocalNow(request?.to)); + const [reason, setReason] = useState(request?.reason ?? ''); + const [substituteId, setSubstituteId] = useState( + request?.substituteId ?? '', + ); + const [halfDayStart, setHalfDayStart] = useState( + request?.halfDayStart ?? false, + ); + const [halfDayEnd, setHalfDayEnd] = useState(request?.halfDayEnd ?? false); const [attachment, setAttachment] = useState(null); const [error, setError] = useState(null); @@ -176,6 +244,21 @@ function NewRequestForm({ onClose }: NewRequestFormProps) { queryFn: () => api.employeeDirectory(), }); + const events = useQuery({ + queryKey: ['request-events', request?.id], + queryFn: () => + request ? api.getRequestEvents(request.id) : Promise.resolve([]), + enabled: !!request, + }); + const revisionNote = + request?.decisionNote ?? + events.data + ?.filter( + (event) => + event.kind === 'Returned' || event.kind === 'SubstituteDeclined', + ) + .at(-1)?.note; + const fromDate = useMemo(() => new Date(`${from}T00:00:00.000Z`), [from]); const year = fromDate.getUTCFullYear(); const balance = useQuery({ @@ -193,7 +276,25 @@ function NewRequestForm({ onClose }: NewRequestFormProps) { }, [isTimeAdjustment, from, to, fromDt, toDt]); const create = useMutation({ - mutationFn: async () => { + mutationFn: async (saveOnly: boolean) => { + if (request) { + const payload = { + expectedUpdatedAt: request.updatedAt, + from: isTimeAdjustment + ? new Date(fromDt).toISOString() + : new Date(`${from}T00:00:00.000Z`).toISOString(), + to: isTimeAdjustment + ? new Date(toDt).toISOString() + : new Date(`${to}T00:00:00.000Z`).toISOString(), + reason: reason || null, + substituteId: type === 'Vacation' ? substituteId || null : null, + halfDayStart: type === 'Vacation' && halfDayStart, + halfDayEnd: type === 'Vacation' && halfDayEnd, + }; + return saveOnly + ? api.updateDraftRequest(request.id, payload) + : api.resubmitRequest(request.id, payload); + } if (type === 'Vacation') { return api.createVacationRequest({ employeeId, @@ -233,19 +334,41 @@ function NewRequestForm({ onClose }: NewRequestFormProps) { const insufficientVacation = type === 'Vacation' && balance.data !== undefined && - balance.data.remainingDays < 1; + balance.data.remainingDays < 0.5; return ( <> - {t('requests.new')} + + {t(request ? 'requests.edit' : 'requests.new')} + {t('requests.chooseTypeRange')}
+

+ {t('requests.employee')}:{' '} + {request?.employeeName || `${user.firstName} ${user.lastName}`} +

+ {request && revisionNote && ( + + + {t('requests.revisionReason')}: {revisionNote} + + + )} + {request && request.workflowState !== 'Draft' && ( + + {t('requests.notDraft')} + + )} + {request?.type === 'SpecialLeave' && ( + + )}
{t('common.cancel')} + {request && ( + + )} diff --git a/docs/ISSUE_FIXES_MANUAL_TEST.de.md b/docs/ISSUE_FIXES_MANUAL_TEST.de.md new file mode 100644 index 0000000..1a10b92 --- /dev/null +++ b/docs/ISSUE_FIXES_MANUAL_TEST.de.md @@ -0,0 +1,209 @@ +# Nachtest der Issue-Pakete + +## Paket #42 – zurückgegebene Anträge bearbeiten + +- Issue: +- Branch: `fix/42-returned-request-edit`, Basis: `9cfbc989cce97bde362821d1e2922951b2abb498`. +- Implementierungs-Worktree: `/Users/patrickschiller/openclockwork-issue-42`. +- Keine Releases, Tags, Versionsänderungen, Merges oder Deployments. Keine neue + Migration erforderlich: Die vorhandenen Felder `updatedAt` und `workflowState` + sichern Schreibzugriffe ab; das vorhandene Ereignis `Resubmitted` wird verwendet. + +### Isoliertes lokales Setup + +Die Grundlage ist [LOCAL_DOCKER_TEST.de.md](LOCAL_DOCKER_TEST.de.md). +Für diesen Nachtest **nicht** die bestehenden Solo-/Team-Projekte wiederverwenden. +Eigene Projekt-, Image-Namen und Port vermeiden Änderungen an anderen Teststacks. +Nur synthetische Daten verwenden; weder Produktivdaten importieren noch eine +vorhandene `.env` laden. + +Vom Implementierungs-Worktree aus arbeiten, oder einen weiteren Test-Worktree +anlegen (der Hauptcheckout mit lokalen Dateien bleibt unberührt): + +```bash +git fetch origin +git worktree add ../openclockwork-42-manual origin/fix/42-returned-request-edit +cd ../openclockwork-42-manual +``` + +Docker und Compose müssen laufen. Der lokale Override liegt außerhalb des Repos: + +```bash +cat > /tmp/openclockwork-42-compose.yml <<'YAML' +services: + api: + image: openclockwork-api:issue42-local + web: + image: openclockwork-web:issue42-local +YAML + +TEST_WEB_PORT=18042 docker compose --env-file /dev/null \ + -p openclockwork-issue42 -f docker-compose.test.yml \ + -f /tmp/openclockwork-42-compose.yml build + +TEST_WEB_PORT=18042 docker compose --env-file /dev/null \ + -p openclockwork-issue42 -f docker-compose.test.yml \ + -f /tmp/openclockwork-42-compose.yml up -d --wait + +TEST_WEB_PORT=18042 docker compose --env-file /dev/null \ + -p openclockwork-issue42 -f docker-compose.test.yml \ + -f /tmp/openclockwork-42-compose.yml exec -T \ + -e NODE_ENV=development \ + -e OPENCLOCKWORK_SEED_CONFIRM_DATABASE=openclockwork_test \ + api node --import tsx prisma/seed.ts +``` + +Seed nur einmal auf dieser leeren Team-Testdatenbank ausführen. Oberfläche: +, Healthcheck: . + +| Rolle | Synthetisches Konto | Verwendung | +| ----------------------- | --------------------------------- | ---------------------------------- | +| Beschäftigte | `anna.mueller@openclockwork.test` | Antragstellerin | +| Führungskraft | `marc.becker@openclockwork.test` | Rückgabe/Genehmigung | +| Vertretung/Fremdzugriff | `bernd.schulz@openclockwork.test` | Ablehnen/Annehmen, kein Eigentümer | +| HR | `hannah.roth@openclockwork.test` | Urlaubskonto und Fremdzugriff | + +Das initiale lokale Seed-Passwort ist `openclockwork`, siehe Basisanleitung. +Separate Browserprofile/private Fenster je Rolle verwenden. Einen freien +Mo–Fr-Zeitraum desselben Jahres ohne Feiertage wählen; vorhandene Seed-Anträge und +Zeitbuchungen nicht überlappen. Für exakte 5/4,5-Tage-Erwartungen müssen die +Arbeitszeitregelung und das Urlaubskonto zu diesem Zeitraum passen. + +### A. Führungskraft gibt zurück → bearbeiten → erneut einreichen + +1. Als Anna unter **Anträge → Neuer Antrag** Urlaub ohne Vertretung für fünf + Arbeitstage beantragen, Grund „Nachtest #42“ eintragen. Annas Name steht in der + Maske und der Liste. In den Browser-Netzwerkdetails die Antwort-ID festhalten. +2. Als Marc unter **Genehmigungen** denselben Antrag prüfen: Annas Name steht im + Eintrag und im **Verlauf**. Notiz „Bitte letzten Tag kürzen“ eingeben und + **Zur Korrektur** wählen. +3. Als Anna Übersicht aktualisieren. **Entwurf · Antrag bearbeiten** mit Maus + oder Tab/Enter öffnen. Erwartet: ursprüngliche Daten, Grund, Halbtagswerte und + Vertretung (falls vorhanden) sind vorbelegt; Annas Name und der Rückgabegrund + erscheinen. Der Antragstyp ist unveränderlich. +4. Letzten Tag oder Halbtagsauswahl ändern. **Entwurf speichern** wählen. + Erwartet: weiterhin Entwurf, kein neuer Antrag, noch kein Genehmigungseingang. + Erneut öffnen: geänderte Werte und Rückgabegrund bleiben vorhanden. +5. **Erneut einreichen** wählen. Erwartet: dieselbe ID wie in Schritt 1; + `workflowState=PendingManager`, aktueller Manager neu zugeordnet; Tageszahl + erneut berechnet (ein halber erster Tag bei fünf Arbeitstagen: 4,5 Tage). + Alte Freigabe-, Entscheidungs-, HR- und Vertretungsannahmefelder sind leer. +6. Als Marc aktualisieren: genau ein Antrag mit neuen Werten. Im **Verlauf** + bleiben Einreichung und Rückgabenotiz erhalten, zusätzlich „Erneut eingereicht“. + Normal genehmigen; Annas Urlaubskonto darf den Antrag nicht doppelt zählen. + +### B. Vertretung lehnt ab → bearbeiten → erneut einreichen + +1. Als Anna neuen Urlaub in einem anderen freien Zeitraum mit Bernd als Vertretung + beantragen. Als Bernd unter **Vertretungen** mit Notiz „Bin selbst weg“ ablehnen. +2. Als Anna den Entwurf öffnen: Werte und Ablehnungsgrund müssen sichtbar sein. + Grund/Zeitraum/Halbtage ändern, Bernd erneut auswählen und erneut einreichen. +3. Erwartet: gleiche ID, `PendingSubstitute`, `substituteAcceptedAt=null`, noch + kein aktueller Manager-Freigabeschritt. Bernd muss erneut ausdrücklich annehmen; + danach folgt Marc. Alternativ Vertretung entfernen: direkt `PendingManager`. +4. Bei einem zurückgegebenen **Sonderurlaub** mit synthetischem PDF dasselbe + durchführen: vorhandener Beleg muss vor/nach der Bearbeitung herunterladbar + bleiben; Historie und Beleg-ID dürfen nicht verloren gehen. +5. Bei **Zeitkorrektur** Uhrzeiten prüfen: gespeicherte lokale Uhrzeiten werden + vorbelegt. Neue Buchungsüberschneidung wird abgewiesen; eine Änderung außerhalb + der Rahmenzeit erfordert weiterhin Sondergenehmigung und HR-Bestätigung. +6. Sprache auf Englisch umstellen: Bearbeiten/Speichern/Erneut einreichen, + Mitarbeiterbeschriftung, Rückgabegrund und Wiedereinreichungsereignis sind + übersetzt, keine neuen rohen i18n-Schlüssel. + +### C. Negative und parallele Fälle + +Die API-Aufrufe lassen sich über die Browser-Netzwerkdetails **des isolierten +Teststacks** wiederholen. Keine echten Bearer-Token in Dokumente/Issues kopieren. + +`GET /api/requests/:id` liefert `updatedAt`. Bearbeiten: +`PATCH /api/requests/:id`; erneut einreichen: `POST /api/requests/:id/resubmit`. +Beide erwarten vollständige editierbare Werte, z. B.: + +```json +{ + "expectedUpdatedAt": "", + "from": "2026-05-04T00:00:00.000Z", + "to": "2026-05-08T00:00:00.000Z", + "reason": "Nachtest", + "substituteId": null, + "halfDayStart": true, + "halfDayEnd": false +} +``` + +Die Beispieldaten an den freien Testzeitraum anpassen. + +| Fall | Erwartung | +| -------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | +| Bernd, Marc oder HR ändern/reichen Annas Entwurf ein | 403; Eigentümer wird ausschließlich aus Bearer-Identität geprüft, nicht aus Body-Feldern | +| Kein Login | 401 | +| Noch ausstehend, genehmigt, abgelehnt oder storniert statt `Draft` | 409; `status=Submitted` allein berechtigt nicht | +| Enddatum vor Anfang, Urlaub über Jahresgrenze, nur Wochenende, ungültige Daten | 400; unveränderter Entwurf | +| Selbstvertretung, nicht vorhandene oder deaktivierte Vertretung | 400 | +| HR reduziert Annas Urlaub unter den neu berechneten Bedarf | 409 bei Speichern und Einreichen; 0,5 verfügbar erlaubt einen einzelnen halben Arbeitstag | +| Überschneidung mit anderem aktivem Antrag oder anderem Entwurf | 409; ausschließlich die eigene ID ist aus der Prüfung ausgenommen | +| Unveränderten zurückgegebenen Urlaub bei exakt ausreichendem Konto einreichen | Erfolg; eigene Reservierung wird weder bei Überschneidung noch beim Saldo doppelt gezählt | +| Entwurf in zwei Tabs öffnen, in A speichern, in B mit altem `updatedAt` speichern/einreichen | 409; kein stilles Überschreiben, Fehler bleibt in der Maske, neu laden und erneut bearbeiten | +| Zweimal gleichzeitig denselben Entwurf einreichen | genau ein Erfolg, ein 409, nur ein `Resubmitted`-Ereignis | +| Zwei verschiedene Entwürfe parallel einreichen, Urlaub reicht nur für einen | genau einer erfolgreich; keine Überbuchung des Kontos | +| Bereits laufende alte Managerentscheidung während Rückgabe + Wiedereinreichung | alte Entscheidung scheitert mit 409, neue Werte/neuer Freigabezyklus bleiben bestehen | + +### Automatisch ausgeführte Gates + +Stand: 07.11.2026. Tests verwenden eine eigene synthetische PostgreSQL-16-Instanz +(`openclockwork_test_42`, Loopback-Port 15442); keine vorhandene Datenbank wird +zurückgesetzt. Die UI-Regressionsprüfungen sind React-Testing-Library-Tests; +der oben beschriebene manuelle Docker-/Browser-Nachtest wurde nicht als bestanden +behauptet. + +Ausgeführt mit Node `v26.12.1`, pnpm `9.15.9` (für diesen Worktree über +`/tmp/openclockwork-42-tools/node_modules/.bin` im `PATH`). + +| Gate | Ergebnis | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------- | +| `pnpm nx e2e api-e2e --testPathPatterns='request-revision\|vacation-workflow\|half-day-vacation\|attachments\|bulk\|carry-over-expiry\|employee-account-access\|work-schedules'` | **75 Tests, 8 Suites bestanden**, einschließlich 20 neuer Revisionsfälle | +| `pnpm nx run-many -t test -p web,shared --run` | **Web: 190 Tests / 33 Dateien**, **Shared: 88 Tests / 7 Dateien bestanden** | +| API-, Web- und Shared-Builds im Lauf `pnpm nx run-many -t build,typecheck,lint -p api,web,shared,api-e2e` | **Alle vorhandenen Build-Targets erfolgreich** | +| Lint im selben Lauf für API, Web, Shared und API-E2E | **0 Fehler**; 13 bestehende Warnungen in unveränderten Dateien | +| `pnpm nx run web:typecheck --force --skip-nx-cache`, danach `pnpm nx run-many -t typecheck -p api,web,shared,api-e2e` | **Alle vier Typechecks erfolgreich** | +| `pnpm verify:api` mit der isolierten Test-`DATABASE_URL` | **Erfolgreich**, OpenAPI und generierter Client identisch zur gestagten Fassung | +| `git diff --check` | **Erfolgreich** | + +Beim kombinierten parallelen Build-/Typecheck-Zwischenlauf entfernte Vite das +von TypeScript ebenfalls verwendete Web-`dist`-Verzeichnis (`TS6305`). Der +anschließende erzwungene, separat ausgeführte Web-Typecheck und der separate +Vier-Projekt-Typecheck bestanden. Deshalb Build und Typecheck für einen Nachlauf +nacheinander starten. Es wurde dafür keine Projektkonfiguration geändert. +Weitere nicht blockierende Ausgaben: React-Router-Future-Hinweise, +Node-localStorage-Hinweis und veraltete Browserslist-Daten. + +Reproduzierbare API-Testumgebung (ausschließlich neue synthetische Testinstanz): + +```bash +docker run --name openclockwork-issue-42-db \ + -e POSTGRES_USER=openclockwork -e POSTGRES_PASSWORD=openclockwork \ + -e POSTGRES_DB=openclockwork_test_42 \ + -p 127.0.0.1:15442:5432 -d postgres:16-alpine + +E2E_DATABASE_URL='postgresql://openclockwork:openclockwork@localhost:15442/openclockwork_test_42?schema=public' \ +TZ=Europe/Berlin pnpm nx e2e api-e2e \ + --testPathPatterns='request-revision|vacation-workflow|half-day-vacation|attachments|bulk|carry-over-expiry|employee-account-access|work-schedules' +``` + +Wenn diese konkrete Testinstanz bereits existiert, nicht ein zweites Mal anlegen. +Die Test-Suite prüft den Testdatenbanknamen vor Migration/Reset. Ein vorhandenes +`DATABASE_URL` muss entfernt oder bewusst auf dieselbe isolierte Instanz gesetzt +werden; widersprüchliche Ziele werden abgewiesen. + +### Stoppen, Daten erhalten + +```bash +TEST_WEB_PORT=18042 docker compose --env-file /dev/null \ + -p openclockwork-issue42 -f docker-compose.test.yml \ + -f /tmp/openclockwork-42-compose.yml stop +``` + +Zum Fortsetzen denselben `up -d --wait`-Befehl verwenden, **nicht** erneut seeden. +Kein `down -v`: Das würde die Nachtestdaten löschen. Folgepakete #41, #43, #45 +und #44 ergänzen diese Anleitung erst bei ihrer jeweiligen Implementierung. From 80c8622a906cff2b28148f83becdbf6bb4c7e9ab Mon Sep 17 00:00:00 2001 From: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> Date: Sat, 7 Nov 2026 02:51:22 +0100 Subject: [PATCH 2/4] fix: preserve correction instants across DST edits (#42) Keep persisted timestamps unchanged until their time field is edited. Resolve wall-clock validation and payloads to identical instants, require explicit ambiguous-time choices, reject clock-change gaps, and cover seconds, both fold occurrences and reason-only save/resubmit cycles. Co-authored-by: patrickschiller <39913537+patrickschiller@users.noreply.github.com> Signed-off-by: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> --- apps/web/src/app/i18n.tsx | 14 ++ apps/web/src/lib/local-date-time.spec.ts | 66 ++++++ apps/web/src/lib/local-date-time.ts | 45 ++++ apps/web/src/routes/RequestsPage.spec.tsx | 269 +++++++++++++++++++++- apps/web/src/routes/RequestsPage.tsx | 167 +++++++++++--- docs/ISSUE_FIXES_MANUAL_TEST.de.md | 85 +++++++ 6 files changed, 608 insertions(+), 38 deletions(-) create mode 100644 apps/web/src/lib/local-date-time.spec.ts create mode 100644 apps/web/src/lib/local-date-time.ts diff --git a/apps/web/src/app/i18n.tsx b/apps/web/src/app/i18n.tsx index eeea78d..2d67e1c 100644 --- a/apps/web/src/app/i18n.tsx +++ b/apps/web/src/app/i18n.tsx @@ -356,6 +356,13 @@ const de: Catalog = { 'requests.notDraft': 'Dieser Antrag ist kein Entwurf mehr. Bitte Übersicht neu laden.', 'requests.loadFailed': 'Antrag konnte nicht geladen werden.', + 'requests.ambiguousTime': + 'Diese Ortszeit kommt bei der Zeitumstellung in {zone} zweimal vor. Bitte das gewünschte Vorkommen wählen; unveränderte Werte behalten das gespeicherte Vorkommen.', + 'requests.timeOccurrence': '{field} – Vorkommen', + 'requests.chooseOccurrence': 'Vorkommen wählen', + 'requests.timeOccurrenceOption': '{number}. Vorkommen ({offset})', + 'requests.nonexistentTime': + 'Bitte eine gültige Ortszeit eingeben. Zeiten in der Lücke der Zeitumstellung existieren nicht.', 'requests.new': 'Neuer Antrag', 'requests.own': 'Eigene Anträge', 'requests.none': 'Noch keine Anträge.', @@ -1036,6 +1043,13 @@ const en: Catalog = { 'requests.notDraft': 'This request is no longer a draft. Please reload the list.', 'requests.loadFailed': 'Could not load request.', + 'requests.ambiguousTime': + 'This local time occurs twice when the clocks change in {zone}. Choose the intended occurrence; unchanged values keep the saved occurrence.', + 'requests.timeOccurrence': '{field} – occurrence', + 'requests.chooseOccurrence': 'Choose occurrence', + 'requests.timeOccurrenceOption': 'Occurrence {number} ({offset})', + 'requests.nonexistentTime': + 'Enter a valid local time. Times in the clock-change gap do not exist.', 'requests.new': 'New request', 'requests.own': 'My requests', 'requests.none': 'No requests yet.', diff --git a/apps/web/src/lib/local-date-time.spec.ts b/apps/web/src/lib/local-date-time.spec.ts new file mode 100644 index 0000000..3d97805 --- /dev/null +++ b/apps/web/src/lib/local-date-time.spec.ts @@ -0,0 +1,66 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + formatLocalDateTime, + localDateTimeCandidates, + utcOffsetLabel, +} from './local-date-time'; + +describe('local date/time resolution', () => { + afterEach(() => vi.unstubAllEnvs()); + + it('preserves seconds and enumerates both Berlin autumn occurrences chronologically', () => { + vi.stubEnv('TZ', 'Europe/Berlin'); + expect(formatLocalDateTime('2026-10-25T01:30:12.345Z')).toBe( + '2026-10-25T02:30:12.345', + ); + expect(localDateTimeCandidates('2026-10-25T02:30:12.345')).toEqual([ + '2026-10-25T00:30:12.345Z', + '2026-10-25T01:30:12.345Z', + ]); + expect(utcOffsetLabel('2026-10-25T00:30:00.000Z')).toBe('UTC+02:00'); + expect(utcOffsetLabel('2026-10-25T01:30:00.000Z')).toBe('UTC+01:00'); + }); + + it.each(['2026-03-29T02:30', '2026-02-30T12:00', '', 'not a date'])( + 'rejects nonexistent or invalid local time %s', + (value) => { + vi.stubEnv('TZ', 'Europe/Berlin'); + expect(localDateTimeCandidates(value)).toEqual([]); + }, + ); + + it('accepts minute, second and fractional precision without rounding', () => { + vi.stubEnv('TZ', 'Europe/Berlin'); + expect(localDateTimeCandidates('2026-06-15T08:10')).toEqual([ + '2026-06-15T06:10:00.000Z', + ]); + expect(localDateTimeCandidates('2026-06-15T08:10:23')).toEqual([ + '2026-06-15T06:10:23.000Z', + ]); + expect(localDateTimeCandidates('2026-06-15T08:10:23.4')).toEqual([ + '2026-06-15T06:10:23.400Z', + ]); + }); + + it('also resolves non-hour folds and gaps', () => { + vi.stubEnv('TZ', 'Australia/Lord_Howe'); + expect(localDateTimeCandidates('2026-04-05T01:45')).toEqual([ + '2026-04-04T14:45:00.000Z', + '2026-04-04T15:15:00.000Z', + ]); + expect(localDateTimeCandidates('2026-10-04T02:15')).toEqual([]); + }); + + it('does not invent ambiguity in UTC and supports negative UTC offsets', () => { + vi.stubEnv('TZ', 'UTC'); + expect(localDateTimeCandidates('2026-10-25T02:30')).toEqual([ + '2026-10-25T02:30:00.000Z', + ]); + vi.stubEnv('TZ', 'America/New_York'); + expect(localDateTimeCandidates('2026-11-01T01:30')).toEqual([ + '2026-11-01T05:30:00.000Z', + '2026-11-01T06:30:00.000Z', + ]); + expect(utcOffsetLabel('2026-11-01T06:30:00.000Z')).toBe('UTC-05:00'); + }); +}); diff --git a/apps/web/src/lib/local-date-time.ts b/apps/web/src/lib/local-date-time.ts new file mode 100644 index 0000000..58b10e8 --- /dev/null +++ b/apps/web/src/lib/local-date-time.ts @@ -0,0 +1,45 @@ +/** datetime-local has no UTC offset. Keep seconds/milliseconds visible and + * enumerate matching instants rather than silently accepting Date's earlier + * occurrence (or its normalization of a nonexistent spring-forward time). */ +export function formatLocalDateTime(iso: string): string { + const d = new Date(iso); + const pad = (n: number, digits = 2) => String(n).padStart(digits, '0'); + return `${pad(d.getFullYear(), 4)}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T${pad(d.getHours())}:${pad(d.getMinutes())}:${pad(d.getSeconds())}.${pad(d.getMilliseconds(), 3)}`; +} + +export function localDateTimeCandidates(value: string): string[] { + const parts = + /^(\d{4}-\d{2}-\d{2}T\d{2}:\d{2})(?::(\d{2})(?:\.(\d{1,3}))?)?$/.exec( + value, + ); + if (!parts) return []; + const normalized = `${parts[1]}:${parts[2] ?? '00'}.${(parts[3] ?? '').padEnd(3, '0')}`; + const parsed = new Date(value); + if ( + !Number.isFinite(parsed.getTime()) || + formatLocalDateTime(parsed.toISOString()) !== normalized + ) + return []; + + // The offsets on the surrounding days include both sides of a clock + // change, including non-hour changes such as Australia/Lord_Howe. + const offsets = new Set( + [-1, 0, 1].map((day) => + new Date(parsed.getTime() + day * 86_400_000).getTimezoneOffset(), + ), + ); + return [...offsets] + .map((offset) => + new Date( + parsed.getTime() + (offset - parsed.getTimezoneOffset()) * 60_000, + ).toISOString(), + ) + .filter((iso) => formatLocalDateTime(iso) === normalized) + .sort(); +} + +export function utcOffsetLabel(iso: string): string { + const minutes = -new Date(iso).getTimezoneOffset(); + const absolute = Math.abs(minutes); + return `UTC${minutes < 0 ? '-' : '+'}${String(Math.floor(absolute / 60)).padStart(2, '0')}:${String(absolute % 60).padStart(2, '0')}`; +} diff --git a/apps/web/src/routes/RequestsPage.spec.tsx b/apps/web/src/routes/RequestsPage.spec.tsx index 6479619..3398af6 100644 --- a/apps/web/src/routes/RequestsPage.spec.tsx +++ b/apps/web/src/routes/RequestsPage.spec.tsx @@ -1,4 +1,12 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { + afterAll, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from 'vitest'; import { fireEvent, screen, waitFor, within } from '@testing-library/react'; import { renderWithProviders } from '../test-utils'; import { RequestsPage } from './RequestsPage'; @@ -41,7 +49,15 @@ vi.mock('../app/auth', () => ({ }), })); -describe('vacation substitute selection', () => { +describe('request creation and draft revision', () => { + const originalTimezone = process.env.TZ; + beforeAll(() => { + process.env.TZ = 'Europe/Berlin'; + }); + afterAll(() => { + if (originalTimezone === undefined) delete process.env.TZ; + else process.env.TZ = originalTimezone; + }); beforeEach(() => { listRequestsMock.mockReset().mockResolvedValue([]); getRequestMock.mockReset(); @@ -208,4 +224,253 @@ describe('vacation substitute selection', () => { screen.queryByRole('button', { name: /Antrag bearbeiten/ }), ).toBeNull(); }); + async function openTimeDraft(from: string, to: string) { + const correction = { ...draft, type: 'TimeAdjustment', from, to }; + listRequestsMock.mockResolvedValue([correction]); + getRequestMock.mockResolvedValue(correction); + renderWithProviders(); + fireEvent.click( + await screen.findByRole('button', { + name: 'Entwurf · Antrag bearbeiten', + }), + ); + await screen.findByLabelText('Von'); + return correction; + } + + it.each([ + ['summer seconds', '2026-06-15T06:10:23.456Z', '2026-06-15T07:20:45.789Z'], + [ + 'first autumn occurrence', + '2026-10-25T00:10:23.456Z', + '2026-10-25T00:30:45.789Z', + ], + [ + 'second autumn occurrence', + '2026-10-25T01:10:23.456Z', + '2026-10-25T01:30:45.789Z', + ], + [ + 'fold crossing with locally earlier end', + '2026-10-25T00:50:23.456Z', + '2026-10-25T01:10:45.789Z', + ], + ])( + 'preserves exact instants on reason-only save and resubmit: %s', + async (_name, from, to) => { + const correction = await openTimeDraft(from, to); + fireEvent.change(screen.getByLabelText('Begründung (optional)'), { + target: { value: 'Reason only' }, + }); + const saved = { + ...correction, + reason: 'Reason only', + updatedAt: '2026-05-01T12:00:00.002Z', + }; + updateDraftRequestMock.mockResolvedValue(saved); + getRequestMock.mockResolvedValue(saved); + const save = screen.getByRole('button', { name: 'Entwurf speichern' }); + expect((save as HTMLButtonElement).disabled).toBe(false); + fireEvent.click(save); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + expect(updateDraftRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ from, to, reason: 'Reason only' }), + ); + fireEvent.click( + screen.getByRole('button', { name: 'Entwurf · Antrag bearbeiten' }), + ); + await screen.findByLabelText('Von'); + const submit = screen.getByRole('button', { name: 'Erneut einreichen' }); + expect((submit as HTMLButtonElement).disabled).toBe(false); + fireEvent.click(submit); + await waitFor(() => + expect(resubmitRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ + from, + to, + reason: 'Reason only', + expectedUpdatedAt: saved.updatedAt, + }), + ), + ); + }, + ); + + it('shows seconds and the saved occurrence, and changing one field preserves the other', async () => { + await openTimeDraft('2026-10-25T01:10:23.456Z', '2026-10-25T02:30:45.789Z'); + expect((screen.getByLabelText('Von') as HTMLInputElement).value).toBe( + '2026-10-25T02:10:23.456', + ); + expect( + (screen.getByLabelText('Von – Vorkommen') as HTMLSelectElement).value, + ).toBe('2026-10-25T01:10:23.456Z'); + fireEvent.change(screen.getByLabelText('Bis'), { + target: { value: '2026-10-25T03:45' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Erneut einreichen' })); + await waitFor(() => + expect(resubmitRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ + from: '2026-10-25T01:10:23.456Z', + to: '2026-10-25T02:45:00.000Z', + }), + ), + ); + }); + + it.each([ + ['2026-10-25T00:30:00.000Z', '1. Vorkommen (UTC+02:00)'], + ['2026-10-25T01:30:00.000Z', '2. Vorkommen (UTC+01:00)'], + ])( + 'requires an explicit occurrence for a changed ambiguous time: %s', + async (from, label) => { + await openTimeDraft( + '2026-10-25T00:10:23.456Z', + '2026-10-25T02:30:45.789Z', + ); + fireEvent.change(screen.getByLabelText('Von'), { + target: { value: '2026-10-25T02:30' }, + }); + const select = screen.getByLabelText('Von – Vorkommen'); + expect((select as HTMLSelectElement).value).toBe(''); + expect( + screen.getByText( + /Diese Ortszeit kommt bei der Zeitumstellung in Europe\/Berlin/, + ), + ).toBeDefined(); + for (const name of ['Entwurf speichern', 'Erneut einreichen']) { + const button = screen.getByRole('button', { name }); + expect((button as HTMLButtonElement).disabled).toBe(true); + fireEvent.click(button); + } + expect(updateDraftRequestMock).not.toHaveBeenCalled(); + expect(resubmitRequestMock).not.toHaveBeenCalled(); + expect(within(select).getByRole('option', { name: label })).toBeDefined(); + fireEvent.change(select, { target: { value: from } }); + fireEvent.click( + screen.getByRole('button', { name: 'Erneut einreichen' }), + ); + await waitFor(() => + expect(resubmitRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ from, to: '2026-10-25T02:30:45.789Z' }), + ), + ); + }, + ); + + it('validates the chosen instants, allows a fold crossing and resets a stale occurrence after another edit', async () => { + await openTimeDraft('2026-10-25T00:30:00.000Z', '2026-10-25T01:40:00.000Z'); + fireEvent.change(screen.getByLabelText('Von'), { + target: { value: '2026-10-25T02:50' }, + }); + fireEvent.change(screen.getByLabelText('Von – Vorkommen'), { + target: { value: '2026-10-25T01:50:00.000Z' }, + }); + const submit = screen.getByRole('button', { name: 'Erneut einreichen' }); + expect((submit as HTMLButtonElement).disabled).toBe(true); + // End 02:40 CET is after start 02:50 CEST, although the wall clock is earlier. + fireEvent.change(screen.getByLabelText('Von – Vorkommen'), { + target: { value: '2026-10-25T00:50:00.000Z' }, + }); + expect((submit as HTMLButtonElement).disabled).toBe(false); + fireEvent.change(screen.getByLabelText('Von'), { + target: { value: '2026-10-25T02:55' }, + }); + expect( + (screen.getByLabelText('Von – Vorkommen') as HTMLSelectElement).value, + ).toBe(''); + expect((submit as HTMLButtonElement).disabled).toBe(true); + fireEvent.change(screen.getByLabelText('Von – Vorkommen'), { + target: { value: '2026-10-25T00:55:00.000Z' }, + }); + fireEvent.click(submit); + await waitFor(() => + expect(resubmitRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ + from: '2026-10-25T00:55:00.000Z', + to: '2026-10-25T01:40:00.000Z', + }), + ), + ); + }); + + it('rejects nonexistent spring-forward and empty times without a request', async () => { + await openTimeDraft('2026-03-29T00:10:23.456Z', '2026-03-29T02:30:45.789Z'); + for (const value of ['2026-03-29T02:30', '']) { + fireEvent.change(screen.getByLabelText('Bis'), { target: { value } }); + expect( + screen.getByText(/Bitte eine gültige Ortszeit eingeben/), + ).toBeDefined(); + const save = screen.getByRole('button', { name: 'Entwurf speichern' }); + const submit = screen.getByRole('button', { name: 'Erneut einreichen' }); + expect((save as HTMLButtonElement).disabled).toBe(true); + expect((submit as HTMLButtonElement).disabled).toBe(true); + fireEvent.click(save); + fireEvent.click(submit); + } + expect(updateDraftRequestMock).not.toHaveBeenCalled(); + expect(resubmitRequestMock).not.toHaveBeenCalled(); + }); + it('offers an accessible English occurrence choice for a changed end time and saves the chosen instant', async () => { + const languageSpy = vi + .spyOn(navigator, 'languages', 'get') + .mockReturnValue(['en-GB']); + try { + const correction = { + ...draft, + type: 'TimeAdjustment', + from: '2026-10-25T00:50:23.456Z', + to: '2026-10-25T02:30:45.789Z', + }; + listRequestsMock.mockResolvedValue([correction]); + getRequestMock.mockResolvedValue(correction); + renderWithProviders(); + fireEvent.click( + await screen.findByRole('button', { name: 'Draft · Edit request' }), + ); + fireEvent.change(await screen.findByLabelText('To'), { + target: { value: '2026-10-25T02:10' }, + }); + expect(screen.getAllByText(/This local time occurs twice/)).toHaveLength( + 2, + ); + const select = screen.getByRole('combobox', { name: 'To – occurrence' }); + expect( + within(select).getByRole('option', { name: 'Choose occurrence' }), + ).toBeDefined(); + expect( + within(select).getByRole('option', { + name: 'Occurrence 1 (UTC+02:00)', + }), + ).toBeDefined(); + expect( + within(select).getByRole('option', { + name: 'Occurrence 2 (UTC+01:00)', + }), + ).toBeDefined(); + const save = screen.getByRole('button', { name: 'Save draft' }); + expect((save as HTMLButtonElement).disabled).toBe(true); + fireEvent.change(select, { + target: { value: '2026-10-25T01:10:00.000Z' }, + }); + expect((save as HTMLButtonElement).disabled).toBe(false); + fireEvent.click(save); + await waitFor(() => + expect(updateDraftRequestMock).toHaveBeenCalledWith( + 'same-request', + expect.objectContaining({ + from: correction.from, + to: '2026-10-25T01:10:00.000Z', + }), + ), + ); + } finally { + languageSpy.mockRestore(); + } + }); }); diff --git a/apps/web/src/routes/RequestsPage.tsx b/apps/web/src/routes/RequestsPage.tsx index f893024..2a26a39 100644 --- a/apps/web/src/routes/RequestsPage.tsx +++ b/apps/web/src/routes/RequestsPage.tsx @@ -18,6 +18,11 @@ import { Label } from '@/components/ui/label'; import { api, type RequestDto, type RequestType } from '../api/client'; import { useCurrentUser } from '../app/auth'; import { useI18n } from '../app/i18n'; +import { + formatLocalDateTime, + localDateTimeCandidates, + utcOffsetLabel, +} from '../lib/local-date-time'; const TYPES: RequestType[] = [ 'Vacation', @@ -209,11 +214,95 @@ interface NewRequestFormProps { request?: RequestDto; } -function isoDateTimeLocalNow(iso?: string): string { - const d = iso ? new Date(iso) : new Date(); - // datetime-local needs YYYY-MM-DDTHH:mm in *local* time - const pad = (n: number) => String(n).padStart(2, '0'); - return `${d.getFullYear()}-${pad(d.getMonth() + 1)}-${pad(d.getDate())}T${pad(d.getHours())}:${pad(d.getMinutes())}`; +function useLocalDateTime(originalIso?: string) { + const [value, setValue] = useState(() => { + const now = new Date(); + now.setSeconds(0, 0); + return formatLocalDateTime(originalIso ?? now.toISOString()); + }); + const [chosenIso, setChosenIso] = useState(originalIso ?? ''); + const candidates = useMemo(() => localDateTimeCandidates(value), [value]); + const chosen = candidates.find( + (iso) => new Date(iso).getTime() === new Date(chosenIso).getTime(), + ); + // Preserve the exact persisted value until this field is changed, not just + // its minute-level local representation. Validation and payload share iso. + const iso = chosen + ? chosenIso + : candidates.length === 1 + ? candidates[0] + : null; + return { + value, + candidates, + iso, + setChosenIso, + change: (next: string) => { + setValue(next); + setChosenIso(''); + }, + }; +} + +function LocalDateTimeField({ + id, + label, + time, +}: { + id: string; + label: string; + time: ReturnType; +}) { + const { t } = useI18n(); + const ambiguous = time.candidates.length > 1; + const invalid = !time.iso; + return ( + <> + time.change(e.target.value)} + aria-invalid={invalid} + aria-describedby={ambiguous || invalid ? `${id}-time-help` : undefined} + /> + {ambiguous ? ( +
+

+ {t('requests.ambiguousTime', { + zone: Intl.DateTimeFormat().resolvedOptions().timeZone, + })} +

+ + +
+ ) : invalid ? ( +

+ {t('requests.nonexistentTime')} +

+ ) : null} + + ); } function NewRequestForm({ onClose, request }: NewRequestFormProps) { @@ -224,10 +313,12 @@ function NewRequestForm({ onClose, request }: NewRequestFormProps) { const [type, setType] = useState(request?.type ?? 'Vacation'); const [from, setFrom] = useState(request?.from.slice(0, 10) ?? today); const [to, setTo] = useState(request?.to.slice(0, 10) ?? today); - const [fromDt, setFromDt] = useState(() => - isoDateTimeLocalNow(request?.from), + const fromTime = useLocalDateTime( + request?.type === 'TimeAdjustment' ? request.from : undefined, + ); + const toTime = useLocalDateTime( + request?.type === 'TimeAdjustment' ? request.to : undefined, ); - const [toDt, setToDt] = useState(() => isoDateTimeLocalNow(request?.to)); const [reason, setReason] = useState(request?.reason ?? ''); const [substituteId, setSubstituteId] = useState( request?.substituteId ?? '', @@ -269,23 +360,35 @@ function NewRequestForm({ onClose, request }: NewRequestFormProps) { const isTimeAdjustment = type === 'TimeAdjustment'; - const invalidRange = useMemo(() => { - if (isTimeAdjustment) - return new Date(toDt).getTime() <= new Date(fromDt).getTime(); - return new Date(to).getTime() < new Date(from).getTime(); - }, [isTimeAdjustment, from, to, fromDt, toDt]); + const unresolvedTime = isTimeAdjustment && (!fromTime.iso || !toTime.iso); + const invalidRange = isTimeAdjustment + ? !fromTime.iso || + !toTime.iso || + new Date(toTime.iso).getTime() <= new Date(fromTime.iso).getTime() + : new Date(to).getTime() < new Date(from).getTime(); const create = useMutation({ mutationFn: async (saveOnly: boolean) => { + const fromIso = isTimeAdjustment + ? fromTime.iso + : new Date(`${from}T00:00:00.000Z`).toISOString(); + const toIso = isTimeAdjustment + ? toTime.iso + : new Date(`${to}T00:00:00.000Z`).toISOString(); + if (!fromIso || !toIso || invalidRange) { + throw new Error( + t( + isTimeAdjustment + ? 'requests.invalidTimeRange' + : 'requests.invalidDateRange', + ), + ); + } if (request) { const payload = { expectedUpdatedAt: request.updatedAt, - from: isTimeAdjustment - ? new Date(fromDt).toISOString() - : new Date(`${from}T00:00:00.000Z`).toISOString(), - to: isTimeAdjustment - ? new Date(toDt).toISOString() - : new Date(`${to}T00:00:00.000Z`).toISOString(), + from: fromIso, + to: toIso, reason: reason || null, substituteId: type === 'Vacation' ? substituteId || null : null, halfDayStart: type === 'Vacation' && halfDayStart, @@ -298,20 +401,14 @@ function NewRequestForm({ onClose, request }: NewRequestFormProps) { if (type === 'Vacation') { return api.createVacationRequest({ employeeId, - from: new Date(`${from}T00:00:00.000Z`).toISOString(), - to: new Date(`${to}T00:00:00.000Z`).toISOString(), + from: fromIso, + to: toIso, substituteId: substituteId || null, reason: reason || null, halfDayStart, halfDayEnd, }); } - const fromIso = isTimeAdjustment - ? new Date(fromDt).toISOString() - : new Date(`${from}T00:00:00.000Z`).toISOString(); - const toIso = isTimeAdjustment - ? new Date(toDt).toISOString() - : new Date(`${to}T00:00:00.000Z`).toISOString(); const created = await api.createRequest({ employeeId, type, @@ -384,11 +481,10 @@ function NewRequestForm({ onClose, request }: NewRequestFormProps) {
{isTimeAdjustment ? ( - setFromDt(e.target.value)} + label={t('common.from')} + time={fromTime} /> ) : ( {isTimeAdjustment ? ( - setToDt(e.target.value)} + label={t('common.to')} + time={toTime} /> ) : ( )} - {invalidRange && ( + {invalidRange && !unresolvedTime && ( {isTimeAdjustment diff --git a/docs/ISSUE_FIXES_MANUAL_TEST.de.md b/docs/ISSUE_FIXES_MANUAL_TEST.de.md index 1a10b92..b79387e 100644 --- a/docs/ISSUE_FIXES_MANUAL_TEST.de.md +++ b/docs/ISSUE_FIXES_MANUAL_TEST.de.md @@ -149,6 +149,59 @@ Die Beispieldaten an den freien Testzeitraum anpassen. | Zwei verschiedene Entwürfe parallel einreichen, Urlaub reicht nur für einen | genau einer erfolgreich; keine Überbuchung des Kontos | | Bereits laufende alte Managerentscheidung während Rückgabe + Wiedereinreichung | alte Entscheidung scheitert mit 409, neue Werte/neuer Freigabezyklus bleiben bestehen | +### D. Zeitkorrekturen: Sekunden und Herbst-Zeitumstellung (#42-Nachkorrektur) + +Den Browser bzw. die Browser-Testumgebung ausdrücklich auf **Europe/Berlin** +stellen (z. B. DevTools → Sensors → Location/Timezone). Die Maske verwendet wie +bisher die lokale Browser-Zeitzone. Nur den isolierten synthetischen Teststack +nutzen, keine vorhandenen Zeitbuchungen überlappen und die Szenarien einzeln +ausführen; anschließend den jeweiligen Testantrag stornieren oder getrennte +synthetische Beschäftigte verwenden. + +Für präzise Ausgangswerte als Anna über die Netzwerkdetails einen synthetischen +`POST /api/requests` mit Annas `employeeId`, `type=TimeAdjustment`, einem +Testgrund und den unten angegebenen `from`-/`to`-Werten anlegen. Der Server +erwartet ISO-Zeitpunkte, nicht offsetlose Ortszeiten. Als Marc zur Korrektur +zurückgeben (nicht genehmigen; die Sondergenehmigungs-/HR-Regeln gelten weiter). + +| Ausgangsfall | ISO-Anfang → ISO-Ende | Lokale Anzeige Europe/Berlin | +| --- | --- | --- | +| Normale Sekunden/Millisekunden | `2026-06-15T06:10:23.456Z` → `2026-06-15T07:20:45.789Z` | 08:10:23.456 → 09:20:45.789 | +| Erstes Herbst-Vorkommen | `2026-10-25T00:10:23.456Z` → `2026-10-25T00:30:45.789Z` | 02:10:23.456 → 02:30:45.789, jeweils UTC+02:00 | +| Zweites Herbst-Vorkommen | `2026-10-25T01:10:23.456Z` → `2026-10-25T01:30:45.789Z` | dieselben Ortszeiten, jeweils UTC+01:00 | +| Über die Rückstellung | `2026-10-25T00:50:23.456Z` → `2026-10-25T01:10:45.789Z` | 02:50:23.456 UTC+02:00 → 02:10:45.789 UTC+01:00 | + +1. Je Ausgangsfall Entwurf öffnen, **nur die Begründung ändern**, speichern, + wieder öffnen und erneut einreichen. Netzwerk-Payload und Antwort für + `PATCH /api/requests/:id` und `POST /api/requests/:id/resubmit` prüfen: + `from`/`to` bleiben **exakt** wie in der Tabelle, einschließlich Sekunden, + Millisekunden und Vorkommen; gleiche ID. Das rückstellungsübergreifende + Intervall bleibt zulässig, obwohl die lokale Enduhrzeit kleiner ist. +2. Zum Vergleich den konkreten Fehlerfall mit + `2026-10-25T01:30:00.000Z` als Anfang und z. B. + `2026-10-25T02:30:00.000Z` als Ende prüfen: Bei einer reinen + Begründungsänderung darf der Anfang **nicht** nach + `2026-10-25T00:30:00.000Z` wechseln. +3. In einem zurückgegebenen Zeitkorrektur-Entwurf den Anfang auf + **25.10.2026 02:30** ändern (Ende z. B. 03:30). Erwartet: verständlicher + Zeitzonenhinweis und beschriftetes Feld **Von – Vorkommen**; kein stiller + Standardwert. Speichern und Wiedereinreichen bleiben gesperrt, bis explizit + gewählt wurde. **1. Vorkommen (UTC+02:00)** sendet + `2026-10-25T00:30:00.000Z`, **2. Vorkommen (UTC+01:00)** sendet + `2026-10-25T01:30:00.000Z`. Das unveränderte Ende bleibt exakt erhalten. +4. Dasselbe am Ende prüfen. Anfang 02:50 UTC+02:00, Ende 02:10 UTC+01:00: + zulässig. Anfang auf das zweite Vorkommen umstellen: Ende liegt davor, + deshalb gesperrt. Erneute Änderung des Ortszeitfelds verwirft eine vorherige + Vorkommenswahl und verlangt bei Mehrdeutigkeit eine neue Auswahl. +5. Ortszeit **29.03.2026 02:30** oder ein leeres Zeitfeld eingeben: + verständlicher Fehler; kein API-Aufruf und keine automatische Verschiebung + auf 03:30. Ein normaler gültiger Zeitpunkt hebt die Sperre wieder auf, + sofern das Ende nach dem Anfang liegt. +6. Auf Englisch wiederholen: **From/To – occurrence**, **Choose occurrence**, + **Occurrence 1 (UTC+02:00)** / **Occurrence 2 (UTC+01:00)** und Erklärung sind + übersetzt. Auswahl mit Tab und Pfeiltasten bedienen; Zeitfeld und Auswahl + sind mit ihren jeweiligen Labels/Hinweisen verknüpft. + ### Automatisch ausgeführte Gates Stand: 07.11.2026. Tests verwenden eine eigene synthetische PostgreSQL-16-Instanz @@ -170,6 +223,38 @@ Ausgeführt mit Node `v26.12.1`, pnpm `9.15.9` (für diesen Worktree über | `pnpm verify:api` mit der isolierten Test-`DATABASE_URL` | **Erfolgreich**, OpenAPI und generierter Client identisch zur gestagten Fassung | | `git diff --check` | **Erfolgreich** | +#### Nachkorrektur des unabhängigen P2-Befunds: Zeit-Roundtrip + +Ergänzt am 07.11.2026, gleicher Branch/Worktree. **18 zusätzliche Regressionen** +(10 UI-Fälle und 8 Zeitauflösungsfälle). Die betroffenen Tests setzen ihre +Zeitzonen selbst und stellen sie wieder her; die Web-Läufe wurden zusätzlich +mit `TZ=UTC` gestartet, damit die Berlin-Fälle nicht zufällig von der +Host-Zeitzone abhängen. + +| Gate | Ergebnis | +| --- | --- | +| Vier neue reine Begründungs-Roundtrips gegen die unveränderte `RequestsPage.tsx` aus `230dbb0` | **4/4 erwartungsgemäß fehlgeschlagen**: Sekundenverlust, zweites Vorkommen um eine Stunde verschoben bzw. gültiges rückstellungsübergreifendes Intervall gesperrt; danach korrigierte Datei wiederhergestellt | +| `TZ=UTC pnpm nx test web --run src/routes/RequestsPage.spec.tsx src/lib/local-date-time.spec.ts src/app/i18n.spec.tsx` | **29/29 bestanden** | +| `TZ=UTC pnpm nx test web --run` (finaler Code) | **208/208 Tests, 34 Dateien bestanden** | +| `TZ=UTC pnpm nx run-many -t test -p web,shared --run` | Web erfolgreich; **Shared 88/88 aus unverändertem Nx-Cache** | +| `pnpm nx run-many -t typecheck,lint -p web` | **Erfolgreich; 0 Fehler, nur 2 bestehende Warnungen** in `ThemeProvider.tsx` und `components/ui/alert.tsx`, keine neue Warnung | +| `pnpm nx build web` (nach beendetem Typecheck) | **Erfolgreich**, einschließlich PWA-Build | +| `git diff --check` | **Erfolgreich** | + +Abgedeckt: erfolgreicher Ablauf Speichern → Wiederöffnen → Wiedereinreichen mit +neuem Versionswert, exakte Sekunden/Millisekunden, beide Herbst-Vorkommen, +unverändertes Gegenfeld, explizite DE/EN-Auswahl beider UTC-Offsets, identische +Validierungs-/Payload-Zeitpunkte, lokal früheres aber tatsächlich späteres Ende, +veraltete Auswahl nach weiterer Zeitänderung, leere Eingabe und Frühjahrslücke. +Die Hilfsfunktion wird zusätzlich für UTC, New York und eine halbstündige +Lord-Howe-Zeitumstellung geprüft. + +Nur Web und Anleitung geändert: keine API-, OpenAPI-, Client-Vertrags- oder +Datenbankänderung. Die bisherigen API-Gates bleiben oben dokumentiert; die +reguläre PR-CI prüft auch auf diesem Head erneut API-Synchronität und API-E2E. +Der manuelle Browser-Durchlauf aus Abschnitt D ist eine Anleitung, **kein** +behauptetes Testergebnis. + Beim kombinierten parallelen Build-/Typecheck-Zwischenlauf entfernte Vite das von TypeScript ebenfalls verwendete Web-`dist`-Verzeichnis (`TS6305`). Der anschließende erzwungene, separat ausgeführte Web-Typecheck und der separate From 4ff247bc18adfc0b245d8dbb044f7968cf2a4678 Mon Sep 17 00:00:00 2001 From: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> Date: Sat, 7 Nov 2026 02:55:13 +0100 Subject: [PATCH 3/4] test: isolate correction editor language preference (#42) Keep the English DST selection regression independent of jsdom persisted locale on Node 20. Verify all 208 web tests with Node 20.20.2 and document the CI follow-up. Co-authored-by: patrickschiller <39913537+patrickschiller@users.noreply.github.com> Signed-off-by: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> --- apps/web/src/routes/RequestsPage.spec.tsx | 12 ++++++++++++ docs/ISSUE_FIXES_MANUAL_TEST.de.md | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/apps/web/src/routes/RequestsPage.spec.tsx b/apps/web/src/routes/RequestsPage.spec.tsx index 3398af6..57004ea 100644 --- a/apps/web/src/routes/RequestsPage.spec.tsx +++ b/apps/web/src/routes/RequestsPage.spec.tsx @@ -1,5 +1,6 @@ import { afterAll, + afterEach, beforeAll, beforeEach, describe, @@ -58,7 +59,18 @@ describe('request creation and draft revision', () => { if (originalTimezone === undefined) delete process.env.TZ; else process.env.TZ = originalTimezone; }); + afterEach(() => vi.restoreAllMocks()); beforeEach(() => { + // Persisted locale wins over navigator.languages. Isolate it on Node 20 + // (jsdom storage) as well as newer Node versions (native localStorage). + vi.spyOn(window, 'localStorage', 'get').mockReturnValue({ + length: 0, + clear: vi.fn(), + key: () => null, + removeItem: vi.fn(), + getItem: () => null, + setItem: vi.fn(), + }); listRequestsMock.mockReset().mockResolvedValue([]); getRequestMock.mockReset(); getRequestEventsMock.mockReset().mockResolvedValue([]); diff --git a/docs/ISSUE_FIXES_MANUAL_TEST.de.md b/docs/ISSUE_FIXES_MANUAL_TEST.de.md index b79387e..073b92d 100644 --- a/docs/ISSUE_FIXES_MANUAL_TEST.de.md +++ b/docs/ISSUE_FIXES_MANUAL_TEST.de.md @@ -249,6 +249,16 @@ veraltete Auswahl nach weiterer Zeitänderung, leere Eingabe und Frühjahrslück Die Hilfsfunktion wird zusätzlich für UTC, New York und eine halbstündige Lord-Howe-Zeitumstellung geprüft. +Der erste CI-Nachlauf bestand 207/208 Web-Tests: Der neue Englisch-Test +übernahm unter Node 20 die in jsdom gespeicherte deutsche Sprachpräferenz, die +absichtlich Vorrang vor der Browsersprache hat. Die Testfixture isoliert den +Storage jetzt explizit wie die vorhandenen i18n-Tests; kein Anwendungscode +musste dafür geändert werden. Der folgende lokale Nachlauf bestand mit +Node **20.20.2** (nur per temporärem Tool-PATH, keine Repository-Konfiguration +geändert): `TZ=UTC pnpm nx test web --run --skip-nx-cache` — +**208/208 Tests in 34 Dateien**. Auch unter Node 26 wurden danach erneut +208/208 bestanden; Lint ist unverändert grün, Typecheck aus unverändertem Cache. + Nur Web und Anleitung geändert: keine API-, OpenAPI-, Client-Vertrags- oder Datenbankänderung. Die bisherigen API-Gates bleiben oben dokumentiert; die reguläre PR-CI prüft auch auf diesem Head erneut API-Synchronität und API-E2E. From dbe68f57a93ba26bdc315d31fd4b313586a4337b Mon Sep 17 00:00:00 2001 From: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> Date: Sat, 7 Nov 2026 03:25:10 +0100 Subject: [PATCH 4/4] fix: approve outside-frame clocked time through manager and HR (#41) Add scoped, versioned TimeEntry review with distinct current-manager and HR stages, durable decision audit, legacy pending support and conservative edit invalidation. Surface employee guidance, reviewer actions and history in German/English, synchronize API contracts, and verify additive database upgrades and security/concurrency regressions. Based on accepted #42; no release or merge. Co-authored-by: patrickschiller <39913537+patrickschiller@users.noreply.github.com> Signed-off-by: Patrick Schiller <39913537+patrickschiller@users.noreply.github.com> --- .../src/api/time-entry-approvals.e2e.spec.ts | 572 +++++++++++++++++ apps/api/openapi.json | 222 +++++++ .../app/terminals/terminal-booking.service.ts | 2 + .../time-entries/time-entries.controller.ts | 47 +- .../app/time-entries/time-entries.module.ts | 3 +- .../app/time-entries/time-entries.service.ts | 64 +- .../time-entries/time-entry-approvals.dto.ts | 71 +++ .../time-entry-approvals.service.ts | 347 +++++++++++ apps/web/src/api/client.ts | 48 ++ apps/web/src/api/generated.ts | 151 +++++ apps/web/src/app/i18n.tsx | 121 ++++ apps/web/src/app/realtime.ts | 2 + .../timeentries/TimeEntryApprovals.spec.tsx | 588 ++++++++++++++++++ .../timeentries/TimeEntryApprovals.tsx | 319 ++++++++++ .../web/src/routes/AdminRequestsPage.spec.tsx | 4 + apps/web/src/routes/AdminRequestsPage.tsx | 3 + apps/web/src/routes/BookingPage.spec.tsx | 50 ++ apps/web/src/routes/BookingPage.tsx | 31 +- docs/ISSUE_FIXES_MANUAL_TEST.de.md | 238 ++++++- ops/upgrade-smoke.mjs | 35 ++ .../migration.sql | 10 + prisma/schema.prisma | 5 + 22 files changed, 2915 insertions(+), 18 deletions(-) create mode 100644 apps/api-e2e/src/api/time-entry-approvals.e2e.spec.ts create mode 100644 apps/api/src/app/time-entries/time-entry-approvals.dto.ts create mode 100644 apps/api/src/app/time-entries/time-entry-approvals.service.ts create mode 100644 apps/web/src/components/timeentries/TimeEntryApprovals.spec.tsx create mode 100644 apps/web/src/components/timeentries/TimeEntryApprovals.tsx create mode 100644 prisma/migrations/20261107031000_time_entry_frame_approval/migration.sql diff --git a/apps/api-e2e/src/api/time-entry-approvals.e2e.spec.ts b/apps/api-e2e/src/api/time-entry-approvals.e2e.spec.ts new file mode 100644 index 0000000..6b8e8a3 --- /dev/null +++ b/apps/api-e2e/src/api/time-entry-approvals.e2e.spec.ts @@ -0,0 +1,572 @@ +import { TimeEntriesService } from '../../../api/src/app/time-entries/time-entries.service'; +import { + createTestApp, + login, + seedEmployee, + type TestContext, +} from '../support/test-app'; + +describe('Outside-frame TimeEntry approvals (#41)', () => { + let ctx: TestContext; + type Person = { id: string; token: string }; + let manager: Person, + otherManager: Person, + owner: Person, + other: Person, + hr: Person, + secondHr: Person; + + beforeAll(async () => { + ctx = await createTestApp(); + }); + afterAll(async () => { + await ctx.close(); + }); + beforeEach(async () => { + await ctx.reset(); + async function person( + name: string, + role: 'Employee' | 'Manager' | 'HRAdmin', + managerId?: string, + ) { + const e = await seedEmployee(ctx.prisma, { + personalNo: name, + firstName: name, + lastName: 'Example', + email: `${name}@test.local`, + role, + managerId, + }); + return { id: e.id, token: await login(ctx.http, e.email) }; + } + manager = await person('manager', 'Manager'); + otherManager = await person('other-manager', 'Manager'); + owner = await person('owner', 'Employee', manager.id); + other = await person('other', 'Employee', otherManager.id); + hr = await person('hr', 'HRAdmin', manager.id); + secondHr = await person('second-hr', 'HRAdmin'); + }); + + async function entry(employeeId = owner.id, extra = {}) { + return ctx.prisma.timeEntry.create({ + data: { + employeeId, + clockIn: new Date('2026-05-04T04:00:00Z'), + clockOut: new Date('2026-05-04T06:00:00Z'), + status: 'Pending', + requiresApproval: true, + source: 'Pwa', + ...extra, + }, + }); + } + function list(who: Person, scope = 'review') { + return ctx.http + .get(`/api/timeentries/approvals?scope=${scope}`) + .set('Authorization', `Bearer ${who.token}`); + } + function decide( + id: string, + who: Person, + revision = 0, + stage = 'Manager', + outcome = 'Approve', + extra = {}, + ) { + return ctx.http + .post(`/api/timeentries/${id}/approval`) + .set('Authorization', `Bearer ${who.token}`) + .send({ revision, stage, outcome, ...extra }); + } + function history(id: string, who: Person) { + return ctx.http + .get(`/api/timeentries/${id}/approval-history`) + .set('Authorization', `Bearer ${who.token}`); + } + async function managerApproved(id: string) { + return (await decide(id, manager).expect(201)).body; + } + + it('reviews legacy Pending/null mode entries through manager then HR with versioned audit and same ID', async () => { + const row = await entry(); + expect(row.approvalMode).toBeNull(); + expect((await list(manager).expect(200)).body[0]).toMatchObject({ + revision: 0, + state: 'PendingManager', + canDecide: true, + managerId: manager.id, + employeeName: 'owner Example', + }); + const managed = await managerApproved(row.id); + expect(managed).toMatchObject({ + revision: 1, + state: 'PendingHr', + canDecide: false, + entry: { id: row.id, status: 'Pending' }, + }); + expect((await list(hr).expect(200)).body[0]).toMatchObject({ + state: 'PendingHr', + canDecide: true, + }); + const final = await decide(row.id, hr, 1, 'HR', 'Approve', { + note: 'HR checked', + }).expect(201); + expect(final.body).toMatchObject({ + revision: 2, + state: 'Approved', + entry: { id: row.id, status: 'Approved', requiresApproval: true }, + }); + expect(await ctx.prisma.timeEntry.count()).toBe(1); + const events = (await history(row.id, owner).expect(200)).body; + expect(events).toMatchObject([ + { + actorId: manager.id, + actorName: 'manager Example', + action: 'FrameManagerApproved', + }, + { actorId: hr.id, action: 'FrameHrConfirmed', reason: 'HR checked' }, + ]); + expect( + events.every( + (e: { occurredAt: string }) => !Number.isNaN(Date.parse(e.occurredAt)), + ), + ).toBe(true); + }); + + it.each(['Manager', 'HR'])( + 'supports %s rejection with mandatory reason and no approval afterward', + async (stage) => { + const row = await entry(); + const actor = stage === 'Manager' ? manager : hr; + const revision = + stage === 'Manager' ? 0 : (await managerApproved(row.id)).revision; + await decide(row.id, actor, revision, stage, 'Reject', { + note: ' ', + }).expect(400); + const rejected = await decide(row.id, actor, revision, stage, 'Reject', { + note: 'Not authorized work', + }).expect(201); + expect(rejected.body).toMatchObject({ + state: 'Rejected', + canDecide: false, + entry: { status: 'Rejected' }, + }); + await decide(row.id, actor, revision + 1, stage).expect(409); + const events = (await history(row.id, owner).expect(200)).body; + expect(events.at(-1)).toMatchObject({ + action: + stage === 'Manager' ? 'FrameManagerRejected' : 'FrameHrRejected', + reason: 'Not authorized work', + }); + }, + ); + + it('scopes review to direct team and own list to bearer identity; employeeId/actorId cannot forge ownership', async () => { + const own = await entry(); + const foreign = await entry(other.id); + expect( + (await list(manager).expect(200)).body.map( + (r: { entry: { id: string } }) => r.entry.id, + ), + ).toEqual([own.id]); + const response = await ctx.http + .get(`/api/timeentries/approvals?scope=own&employeeId=${other.id}`) + .set('Authorization', `Bearer ${owner.token}`) + .expect(200); + expect( + response.body.map((r: { entry: { id: string } }) => r.entry.id), + ).toEqual([own.id]); + await list(owner).expect(403); + await decide(own.id, otherManager, 0, 'Manager', 'Approve', { + actorId: manager.id, + employeeId: owner.id, + }).expect(403); + await decide(foreign.id, manager).expect(403); + await history(foreign.id, manager).expect(403); + await history(own.id, other).expect(403); + await decide(own.id, owner).expect(403); + await list(owner, 'invalid').expect(400); + await ctx.http.get('/api/timeentries/approvals').expect(401); + await ctx.http + .post(`/api/timeentries/${own.id}/approval`) + .send({ revision: 0, stage: 'Manager', outcome: 'Approve' }) + .expect(401); + }); + + it('does not let HR skip manager stage, managers act as HR, or owners self-authorize through a broad role', async () => { + const row = await entry(); + await decide(row.id, hr, 0, 'HR').expect(409); + await decide(row.id, hr, 0, 'Manager').expect(403); + await decide(row.id, manager, 0, 'HR').expect(403); + await managerApproved(row.id); + await decide(row.id, manager, 1, 'HR').expect(403); + const ownManager = await entry(manager.id); + await ctx.prisma.employee.update({ + where: { id: manager.id }, + data: { managerId: manager.id }, + }); + await decide(ownManager.id, manager).expect(403); + const ownHr = await entry(hr.id); + await decide(ownHr.id, manager).expect(201); + await decide(ownHr.id, hr, 1, 'HR').expect(403); + await decide(ownHr.id, secondHr, 1, 'HR').expect(201); + expect( + (await list(hr).expect(200)).body.some( + (r: { entry: { id: string } }) => r.entry.id === ownHr.id, + ), + ).toBe(false); + }); + + it.each(['missing', 'inactive', 'wrong-role', 'self'])( + 'blocks %s manager configuration until HR assigns a valid manager', + async (kind) => { + const row = await entry(); + if (kind === 'missing') + await ctx.prisma.employee.update({ + where: { id: owner.id }, + data: { managerId: null }, + }); + if (kind === 'inactive') + await ctx.prisma.employee.update({ + where: { id: manager.id }, + data: { isActive: false }, + }); + if (kind === 'wrong-role') + await ctx.prisma.employee.update({ + where: { id: owner.id }, + data: { managerId: hr.id }, + }); + if (kind === 'self') + await ctx.prisma.employee.update({ + where: { id: owner.id }, + data: { managerId: owner.id }, + }); + expect((await list(owner, 'own').expect(200)).body[0]).toMatchObject({ + state: 'BlockedManager', + canDecide: false, + }); + await decide(row.id, hr, 0, 'HR').expect(409); + await ctx.prisma.employee.update({ + where: { id: owner.id }, + data: { managerId: otherManager.id }, + }); + const response = await decide(row.id, otherManager).expect(201); + expect(response.body.state).toBe('PendingHr'); + await decide(row.id, hr, 1, 'HR').expect(201); + }, + ); + + it('manager reassignment invalidates the pending HR stage and denies the old manager and stale HR tab', async () => { + const row = await entry(); + await managerApproved(row.id); + await ctx.prisma.employee.update({ + where: { id: owner.id }, + data: { managerId: otherManager.id }, + }); + expect((await list(hr).expect(200)).body[0]).toMatchObject({ + state: 'PendingManager', + canDecide: false, + }); + await decide(row.id, hr, 1, 'HR').expect(409); + await decide(row.id, manager, 1).expect(403); + await decide(row.id, otherManager, 1).expect(201); + await decide(row.id, hr, 2, 'HR').expect(201); + }); + + it('manager role change invalidates their sign-off and cannot promote the same person to HR confirmer', async () => { + const row = await entry(); + await managerApproved(row.id); + await ctx.prisma.employee.update({ + where: { id: manager.id }, + data: { role: 'HRAdmin' }, + }); + await decide(row.id, manager, 1, 'HR').expect(409); + expect((await list(owner, 'own').expect(200)).body[0].state).toBe( + 'BlockedManager', + ); + }); + + it.each(['Open', 'Approved', 'Rejected', 'Voided', 'InFrame', 'Solo'])( + 'cannot approve %s entries', + async (kind) => { + const row = await entry(owner.id, { + ...(kind === 'Open' ? { clockOut: null, status: 'Open' } : {}), + ...(kind === 'Approved' || kind === 'Rejected' ? { status: kind } : {}), + ...(kind === 'Voided' ? { voidedAt: new Date() } : {}), + ...(kind === 'InFrame' + ? { requiresApproval: false, status: 'Approved' } + : {}), + ...(kind === 'Solo' ? { approvalMode: 'Solo' } : {}), + }); + await decide(row.id, manager).expect(409); + if (kind === 'InFrame' || kind === 'Solo') + expect((await list(manager).expect(200)).body).toEqual([]); + }, + ); + + it('requires exact revision/stage and only one parallel manager decision succeeds', async () => { + const row = await entry(); + const replies = await Promise.all([ + decide(row.id, manager), + decide(row.id, manager), + ]); + expect(replies.map((r) => r.status).sort()).toEqual([201, 409]); + await decide(row.id, manager, 0).expect(409); + expect( + await ctx.prisma.timeEntryAudit.count({ where: { timeEntryId: row.id } }), + ).toBe(1); + const hrs = await Promise.all([ + decide(row.id, hr, 1, 'HR'), + decide(row.id, secondHr, 1, 'HR'), + ]); + expect(hrs.map((r) => r.status).sort()).toEqual([201, 409]); + expect( + await ctx.prisma.timeEntryAudit.count({ where: { timeEntryId: row.id } }), + ).toBe(2); + }); + + it('validates missing/bad versions and enums without modifying the entry', async () => { + const row = await entry(); + for (const body of [ + { stage: 'Manager', outcome: 'Approve' }, + { revision: -1, stage: 'Manager', outcome: 'Approve' }, + { revision: 0, stage: 'Employee', outcome: 'Approve' }, + ]) { + await ctx.http + .post(`/api/timeentries/${row.id}/approval`) + .set('Authorization', `Bearer ${manager.token}`) + .send(body) + .expect(400); + } + expect( + (await ctx.prisma.timeEntry.findUniqueOrThrow({ where: { id: row.id } })) + .revision, + ).toBe(0); + }); + + it('editing manager-approved or HR-confirmed booking restarts manager review and audits invalidation', async () => { + const row = await entry(); + await managerApproved(row.id); + const edit = () => + ctx.http + .patch(`/api/timeentries/${row.id}`) + .set('Authorization', `Bearer ${owner.token}`) + .send({ activity: 'Corrected description' }) + .expect(200); + await edit(); + await decide(row.id, hr, 1, 'HR').expect(409); + expect((await list(owner, 'own').expect(200)).body[0]).toMatchObject({ + state: 'PendingManager', + revision: 2, + }); + await decide(row.id, manager, 2).expect(201); + await decide(row.id, hr, 3, 'HR').expect(201); + await edit(); + expect((await list(owner, 'own').expect(200)).body[0]).toMatchObject({ + state: 'PendingManager', + revision: 5, + entry: { status: 'Pending' }, + }); + expect( + (await history(row.id, owner).expect(200)).body.filter( + (r: { action: string }) => r.action === 'FrameApprovalInvalidated', + ), + ).toHaveLength(2); + }); + + it('serializes an edit against approval without allowing stale decisions to corrupt the result', async () => { + const row = await entry(); + const replies = await Promise.all([ + decide(row.id, manager), + ctx.http + .patch(`/api/timeentries/${row.id}`) + .set('Authorization', `Bearer ${owner.token}`) + .send({ activity: 'New activity', revision: 0 }), + ]); + expect(replies.filter((r) => r.status === 409)).toHaveLength(1); + expect( + replies.filter((r) => r.status === 200 || r.status === 201), + ).toHaveLength(1); + const current = await ctx.prisma.timeEntry.findUniqueOrThrow({ + where: { id: row.id }, + }); + expect(current.revision).toBe(1); + expect(await ctx.prisma.timeEntryAudit.count()).toBe(1); + }); + + it('split invalidates prior sign-off, recalculates frame per segment, and never launders a rejection', async () => { + const row = await entry(); + await managerApproved(row.id); + const split = await ctx.http + .post(`/api/timeentries/${row.id}/split`) + .set('Authorization', `Bearer ${owner.token}`) + .send({ at: '2026-05-04T05:00:00Z' }) + .expect(201); + expect(split.body.first).toMatchObject({ + requiresApproval: true, + status: 'Pending', + }); + expect(split.body.second).toMatchObject({ + requiresApproval: false, + status: 'Approved', + }); + expect( + (await list(manager).expect(200)).body.find( + (item: { entry: { id: string } }) => item.entry.id === row.id, + ).state, + ).toBe('PendingManager'); + await decide(row.id, manager, 2, 'Manager', 'Reject', { + note: 'No', + }).expect(201); + const again = await ctx.http + .post(`/api/timeentries/${row.id}/split`) + .set('Authorization', `Bearer ${owner.token}`) + .send({ at: '2026-05-04T04:30:00Z' }) + .expect(201); + expect(again.body.first.status).toBe('Rejected'); + expect(again.body.second.status).toBe('Rejected'); + }); + + it('closed PWA booking computes actual frame and enters manager review', async () => { + // Controlled clock-out via the service; production controller calls same method. + const service = ctx.app.get(TimeEntriesService); + const schedule = await ctx.prisma.workSchedule.create({ + data: { name: 'Narrow', frameStart: '07:00', frameEnd: '08:00' }, + }); + await ctx.prisma.employee.update({ + where: { id: owner.id }, + data: { workScheduleId: schedule.id }, + }); + const start = new Date(); + start.setDate(start.getDate() - 1); + start.setHours(6, 0, 0, 0); + const open = await entry(owner.id, { + clockIn: start, + clockOut: null, + status: 'Open', + requiresApproval: true, + }); + expect((await list(manager).expect(200)).body[0]).toMatchObject({ + state: 'Open', + canDecide: false, + }); + const closed = await service.clockOut(owner.id); + expect(closed).toMatchObject({ + id: open.id, + requiresApproval: true, + status: 'Pending', + }); + expect((await list(manager).expect(200)).body[0].state).toBe( + 'PendingManager', + ); + }); + + it('leaves ordinary in-frame and Solo creation unchanged and does not list them for review', async () => { + await entry(owner.id, { + clockIn: new Date('2026-05-04T06:00:00Z'), + clockOut: new Date('2026-05-04T14:00:00Z'), + status: 'Approved', + requiresApproval: false, + }); + await entry(owner.id, { + approvalMode: 'Solo', + status: 'Approved', + requiresApproval: false, + }); + expect((await list(manager).expect(200)).body).toEqual([]); + expect((await list(owner, 'own').expect(200)).body).toEqual([]); + }); + + it('project-range segmentation invalidates a prior manager sign-off for every outside-frame segment', async () => { + const row = await entry(); + await managerApproved(row.id); + const project = await ctx.prisma.project.create({ + data: { code: 'FRAME', name: 'Synthetic project' }, + }); + await ctx.prisma.projectAssignment.create({ + data: { projectId: project.id, employeeId: owner.id }, + }); + await ctx.http + .post('/api/timeentries/book-project') + .set('Authorization', `Bearer ${owner.token}`) + .send({ + employeeId: owner.id, + from: '2026-05-04T04:15:00Z', + to: '2026-05-04T05:30:00Z', + projectId: project.id, + }) + .expect(201); + const rows = (await list(manager).expect(200)).body; + expect(rows).toHaveLength(3); + const pending = rows.filter( + (item: { entry: { requiresApproval: boolean } }) => + item.entry.requiresApproval, + ); + expect(pending).toHaveLength(2); + expect( + pending.every( + (item: { state: string }) => item.state === 'PendingManager', + ), + ).toBe(true); + expect( + rows.filter((item: { state: string }) => item.state === 'Approved'), + ).toHaveLength(1); + await decide(row.id, hr, 1, 'HR').expect(409); + const invalidations = await ctx.prisma.timeEntryAudit.findMany({ + where: { action: { startsWith: 'Frame' } }, + }); + expect( + invalidations.some( + (event) => event.action === 'FrameApprovalInvalidated', + ), + ).toBe(true); + }); + + it('retains discoverable decision history when a split moves the original ID wholly inside the frame', async () => { + const row = await entry(owner.id, { + clockIn: new Date('2026-05-04T06:00:00Z'), + clockOut: new Date('2026-05-04T22:00:00Z'), + }); + await managerApproved(row.id); + await ctx.http + .post(`/api/timeentries/${row.id}/split`) + .set('Authorization', `Bearer ${owner.token}`) + .send({ at: '2026-05-04T20:00:00Z' }) + .expect(201); + const views = (await list(owner, 'own').expect(200)).body; + expect(views).toHaveLength(2); + expect( + views.find((item: { entry: { id: string } }) => item.entry.id === row.id), + ).toMatchObject({ + state: 'Approved', + canDecide: false, + entry: { requiresApproval: false }, + }); + const events = (await history(row.id, owner).expect(200)).body; + expect(events.map((event: { action: string }) => event.action)).toEqual([ + 'FrameManagerApproved', + 'FrameApprovalInvalidated', + ]); + await decide(row.id, manager, 2).expect(409); + }); + + it('Solo installation cannot expose or approve archived Team entries', async () => { + const row = await entry(); + await ctx.prisma.installationSettings.create({ + data: { + id: 1, + mode: 'Solo', + ownerEmployeeId: hr.id, + setupCompleted: true, + }, + }); + await list(hr).expect(403); + await list(hr, 'own').expect(403); + await decide(row.id, hr, 0, 'HR').expect(403); + await history(row.id, hr).expect(403); + expect( + (await ctx.prisma.timeEntry.findUniqueOrThrow({ where: { id: row.id } })) + .status, + ).toBe('Pending'); + }); +}); diff --git a/apps/api/openapi.json b/apps/api/openapi.json index 960390a..0b2fcdb 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -2027,6 +2027,122 @@ "tags": ["time-entries"] } }, + "/api/timeentries/approvals": { + "get": { + "operationId": "TimeEntriesController_approvalList", + "parameters": [ + { + "name": "scope", + "required": false, + "in": "query", + "schema": { + "enum": ["own", "review"], + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/TimeEntryApprovalDto" + } + } + } + } + } + }, + "security": [ + { + "bearer": [] + } + ], + "tags": ["time-entries"] + } + }, + "/api/timeentries/{id}/approval-history": { + "get": { + "operationId": "TimeEntriesController_approvalHistory", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "", + "content": { + "application/json": { + "schema": { + "type": "array", + "items": { + "$ref": "#/components/schemas/TimeEntryApprovalEventDto" + } + } + } + } + } + }, + "security": [ + { + "bearer": [] + } + ], + "tags": ["time-entries"] + } + }, + "/api/timeentries/{id}/approval": { + "post": { + "operationId": "TimeEntriesController_decideApproval", + "parameters": [ + { + "name": "id", + "required": true, + "in": "path", + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DecideTimeEntryApprovalDto" + } + } + } + }, + "responses": { + "201": { + "description": "", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/TimeEntryApprovalDto" + } + } + } + } + }, + "security": [ + { + "bearer": [] + } + ], + "tags": ["time-entries"] + } + }, "/api/timeentries/clock-in": { "post": { "operationId": "TimeEntriesController_clockIn", @@ -6523,6 +6639,112 @@ "summary" ] }, + "TimeEntryApprovalDto": { + "type": "object", + "properties": { + "entry": { + "$ref": "#/components/schemas/TimeEntryDto" + }, + "revision": { + "type": "number", + "minimum": 0 + }, + "employeeName": { + "type": "string" + }, + "managerId": { + "type": "string", + "nullable": true, + "format": "uuid" + }, + "managerName": { + "type": "string", + "nullable": true + }, + "state": { + "type": "string", + "enum": [ + "Open", + "BlockedManager", + "PendingManager", + "PendingHr", + "Approved", + "Rejected", + "Voided" + ] + }, + "canDecide": { + "type": "boolean" + } + }, + "required": [ + "entry", + "revision", + "employeeName", + "managerId", + "managerName", + "state", + "canDecide" + ] + }, + "TimeEntryApprovalEventDto": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "actorId": { + "type": "string", + "nullable": true, + "format": "uuid" + }, + "actorName": { + "type": "string", + "nullable": true + }, + "action": { + "type": "string" + }, + "reason": { + "type": "string", + "nullable": true + }, + "occurredAt": { + "type": "string", + "format": "date-time" + } + }, + "required": [ + "id", + "actorId", + "actorName", + "action", + "reason", + "occurredAt" + ] + }, + "DecideTimeEntryApprovalDto": { + "type": "object", + "properties": { + "revision": { + "type": "number", + "minimum": 0 + }, + "stage": { + "type": "string", + "enum": ["Manager", "HR"] + }, + "outcome": { + "type": "string", + "enum": ["Approve", "Reject"] + }, + "note": { + "type": "string", + "maxLength": 2000 + } + }, + "required": ["revision", "stage", "outcome"] + }, "ClockInDto": { "type": "object", "properties": { diff --git a/apps/api/src/app/terminals/terminal-booking.service.ts b/apps/api/src/app/terminals/terminal-booking.service.ts index f91e7b7..a67f7b7 100644 --- a/apps/api/src/app/terminals/terminal-booking.service.ts +++ b/apps/api/src/app/terminals/terminal-booking.service.ts @@ -105,6 +105,8 @@ export async function bookTerminalTime( revision: { increment: 1 }, status: requires ? 'Pending' : 'Approved', requiresApproval: requires, + frameApproverId: null, + frameApprovedRevision: null, clockOutLatitude: position?.latitude ?? null, clockOutLongitude: position?.longitude ?? null, clockOutAccuracyMeters: position?.accuracyMeters ?? null, diff --git a/apps/api/src/app/time-entries/time-entries.controller.ts b/apps/api/src/app/time-entries/time-entries.controller.ts index 3965574..11cb3bb 100644 --- a/apps/api/src/app/time-entries/time-entries.controller.ts +++ b/apps/api/src/app/time-entries/time-entries.controller.ts @@ -14,10 +14,17 @@ import { ApiOkResponse, ApiCreatedResponse, ApiTags, + ApiQuery, } from '@nestjs/swagger'; import { JwtAuthGuard } from '../auth/jwt-auth.guard'; import { CurrentUser } from '../auth/current-user.decorator'; import type { JwtUser } from '../auth/jwt.strategy'; +import { TimeEntryApprovalsService } from './time-entry-approvals.service'; +import { + DecideTimeEntryApprovalDto, + TimeEntryApprovalDto, + TimeEntryApprovalEventDto, +} from './time-entry-approvals.dto'; import { TimeEntriesService } from './time-entries.service'; import { BookProjectRangeDto, @@ -40,7 +47,10 @@ import { @ApiTags('time-entries') @Controller('timeentries') export class TimeEntriesController { - constructor(private readonly entries: TimeEntriesService) {} + constructor( + private readonly entries: TimeEntriesService, + private readonly approvals: TimeEntryApprovalsService, + ) {} @Get() @ApiOkResponse({ type: TimeEntryDto, isArray: true }) @@ -60,6 +70,41 @@ export class TimeEntriesController { ); } + @Get('approvals') + @ApiQuery({ name: 'scope', required: false, enum: ['own', 'review'] }) + @ApiOkResponse({ type: TimeEntryApprovalDto, isArray: true }) + @ApiBearerAuth() + @UseGuards(JwtAuthGuard) + approvalList( + @CurrentUser() user: JwtUser, + @Query('scope') scope?: string, + ): Promise { + return this.approvals.list(user.id, scope); + } + + @Get(':id/approval-history') + @ApiOkResponse({ type: TimeEntryApprovalEventDto, isArray: true }) + @ApiBearerAuth() + @UseGuards(JwtAuthGuard) + approvalHistory( + @Param('id', new ParseUUIDPipe()) id: string, + @CurrentUser() user: JwtUser, + ): Promise { + return this.approvals.history(id, user); + } + + @Post(':id/approval') + @ApiCreatedResponse({ type: TimeEntryApprovalDto }) + @ApiBearerAuth() + @UseGuards(JwtAuthGuard) + decideApproval( + @Param('id', new ParseUUIDPipe()) id: string, + @Body() dto: DecideTimeEntryApprovalDto, + @CurrentUser() user: JwtUser, + ): Promise { + return this.approvals.decide(id, dto, user); + } + @Post('clock-in') @ApiCreatedResponse({ type: TimeEntryDto }) @ApiBearerAuth() diff --git a/apps/api/src/app/time-entries/time-entries.module.ts b/apps/api/src/app/time-entries/time-entries.module.ts index 903233e..18f4328 100644 --- a/apps/api/src/app/time-entries/time-entries.module.ts +++ b/apps/api/src/app/time-entries/time-entries.module.ts @@ -1,12 +1,13 @@ import { Module } from '@nestjs/common'; import { AuthModule } from '../auth/auth.module'; import { TimeEntriesController } from './time-entries.controller'; +import { TimeEntryApprovalsService } from './time-entry-approvals.service'; import { TimeEntriesService } from './time-entries.service'; @Module({ imports: [AuthModule], controllers: [TimeEntriesController], - providers: [TimeEntriesService], + providers: [TimeEntriesService, TimeEntryApprovalsService], exports: [TimeEntriesService], }) export class TimeEntriesModule {} diff --git a/apps/api/src/app/time-entries/time-entries.service.ts b/apps/api/src/app/time-entries/time-entries.service.ts index 27a201f..5e5cbf2 100644 --- a/apps/api/src/app/time-entries/time-entries.service.ts +++ b/apps/api/src/app/time-entries/time-entries.service.ts @@ -681,6 +681,8 @@ export class TimeEntriesService { clockOut: bookingNow, status: requires ? 'Pending' : 'Approved', requiresApproval: requires, + frameApproverId: null, + frameApprovedRevision: null, revision: { increment: 1 }, clockOutLatitude: policy.isSolo && !policy.gpsEnabled @@ -808,9 +810,32 @@ export class TimeEntriesService { const before = await this.auditSnapshot(tx, current); const row = await tx.timeEntry.update({ where: { id }, - data: { ...data, revision: { increment: 1 } }, + data: { + ...data, + revision: { increment: 1 }, + ...(current.approvalMode !== 'Solo' && current.requiresApproval + ? { + frameApproverId: null, + frameApprovedRevision: null, + ...(!current.voidedAt && + current.clockOut && + current.status !== 'Rejected' + ? { status: 'Pending' as const } + : {}), + } + : {}), + }, include: ENTRY_INCLUDE, }); + if (current.approvalMode !== 'Solo' && current.requiresApproval) + await this.writeAudit( + tx, + row, + user.id, + 'FrameApprovalInvalidated', + before, + null, + ); if (isSolo || current.approvalMode === 'Solo') await this.writeAudit(tx, row, user.id, 'BookingUpdated', before, null); return row; @@ -911,6 +936,8 @@ export class TimeEntriesService { captureGroupId, revision: { increment: 1 }, requiresApproval: firstRequires, + frameApproverId: null, + frameApprovedRevision: null, status: statusFor(firstRequires), clockOutLatitude: null, clockOutLongitude: null, @@ -956,6 +983,24 @@ export class TimeEntriesService { where: { timeEntryId: entry.id, action: 'clock-out' }, data: { timeEntryId: secondSegment.id }, }); + if (entry.approvalMode !== 'Solo' && entry.requiresApproval) { + await this.writeAudit( + tx, + firstSegment, + user.id, + 'FrameApprovalInvalidated', + before, + null, + ); + await this.writeAudit( + tx, + secondSegment, + user.id, + 'FrameSegmentCreated', + null, + null, + ); + } if (isSolo || entry.approvalMode === 'Solo') { await this.writeAudit( tx, @@ -1097,6 +1142,8 @@ export class TimeEntriesService { clockOut, captureGroupId, requiresApproval, + frameApproverId: null, + frameApprovedRevision: null, status: requiresApproval ? ('Pending' as const) : ('Approved' as const), @@ -1132,6 +1179,19 @@ export class TimeEntriesService { where: { timeEntryId: entry.id, action: 'clock-out' }, data: { timeEntryId: last.id }, }); + if (entry.approvalMode !== 'Solo' && entry.requiresApproval) { + for (const segment of segments) + await this.writeAudit( + tx, + segment, + user.id, + segment.id === entry.id + ? 'FrameApprovalInvalidated' + : 'FrameSegmentCreated', + segment.id === entry.id ? before : null, + null, + ); + } if (isSolo || entry.approvalMode === 'Solo') { for (const segment of segments) await this.writeAudit( @@ -1725,6 +1785,8 @@ export class TimeEntriesService { status: entry.status, requiresApproval: entry.requiresApproval, approvalMode: entry.approvalMode, + frameApproverId: entry.frameApproverId, + frameApprovedRevision: entry.frameApprovedRevision, captureGroupId: entry.captureGroupId, breakRules: parseBreakRules(entry.breakRules), voidedAt: entry.voidedAt?.toISOString() ?? null, diff --git a/apps/api/src/app/time-entries/time-entry-approvals.dto.ts b/apps/api/src/app/time-entries/time-entry-approvals.dto.ts new file mode 100644 index 0000000..2decd4c --- /dev/null +++ b/apps/api/src/app/time-entries/time-entry-approvals.dto.ts @@ -0,0 +1,71 @@ +import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; +import { + IsIn, + IsInt, + IsOptional, + IsString, + MaxLength, + Min, +} from 'class-validator'; +import { TimeEntryDto } from './time-entries.dto'; + +export const ENTRY_APPROVAL_STATES = [ + 'Open', + 'BlockedManager', + 'PendingManager', + 'PendingHr', + 'Approved', + 'Rejected', + 'Voided', +] as const; +export type EntryApprovalState = (typeof ENTRY_APPROVAL_STATES)[number]; + +export class TimeEntryApprovalDto { + @ApiProperty({ type: TimeEntryDto }) + entry!: TimeEntryDto; + @ApiProperty({ minimum: 0 }) + revision!: number; + @ApiProperty() + employeeName!: string; + @ApiProperty({ type: String, nullable: true, format: 'uuid' }) + managerId!: string | null; + @ApiProperty({ type: String, nullable: true }) + managerName!: string | null; + @ApiProperty({ enum: ENTRY_APPROVAL_STATES }) + state!: EntryApprovalState; + @ApiProperty() + canDecide!: boolean; +} + +export class DecideTimeEntryApprovalDto { + @ApiProperty({ minimum: 0 }) + @IsInt() + @Min(0) + revision!: number; + @ApiProperty({ enum: ['Manager', 'HR'] }) + @IsIn(['Manager', 'HR']) + stage!: 'Manager' | 'HR'; + @ApiProperty({ enum: ['Approve', 'Reject'] }) + @IsIn(['Approve', 'Reject']) + outcome!: 'Approve' | 'Reject'; + @ApiPropertyOptional({ maxLength: 2000 }) + @IsOptional() + @IsString() + @MaxLength(2000) + note?: string; +} + +export class TimeEntryApprovalEventDto { + @ApiProperty() + id!: string; + @ApiProperty({ type: String, nullable: true, format: 'uuid' }) + actorId!: string | null; + @ApiProperty({ type: String, nullable: true }) + actorName!: string | null; + @ApiProperty() + action!: string; + @ApiProperty({ type: String, nullable: true }) + reason!: string | null; + @ApiProperty({ format: 'date-time' }) + occurredAt!: string; +} diff --git a/apps/api/src/app/time-entries/time-entry-approvals.service.ts b/apps/api/src/app/time-entries/time-entry-approvals.service.ts new file mode 100644 index 0000000..a0ab33c --- /dev/null +++ b/apps/api/src/app/time-entries/time-entry-approvals.service.ts @@ -0,0 +1,347 @@ +import { + BadRequestException, + ConflictException, + ForbiddenException, + Injectable, + NotFoundException, +} from '@nestjs/common'; +import { Prisma, type Employee } from '@prisma/client'; +import type { JwtUser } from '../auth/jwt.strategy'; +import { PrismaService } from '../prisma/prisma.service'; +import { EventsGateway } from '../events/events.gateway'; +import { toTimeEntryDto } from './time-entries.dto'; +import { + type DecideTimeEntryApprovalDto, + type EntryApprovalState, + type TimeEntryApprovalDto, + type TimeEntryApprovalEventDto, +} from './time-entry-approvals.dto'; + +const APPROVAL_INCLUDE = { + project: { select: { code: true, name: true } }, + serviceOrder: { select: { orderNo: true, title: true } }, + employee: { + select: { + id: true, + firstName: true, + lastName: true, + isActive: true, + managerId: true, + manager: { + select: { + id: true, + firstName: true, + lastName: true, + role: true, + isActive: true, + }, + }, + }, + }, +} as const; +type ReviewEntry = Prisma.TimeEntryGetPayload<{ + include: typeof APPROVAL_INCLUDE; +}>; + +function validManager(row: ReviewEntry): boolean { + const manager = row.employee.manager; + return ( + !!manager?.isActive && + manager.role === 'Manager' && + manager.id !== row.employeeId + ); +} + +export function frameApprovalState(row: ReviewEntry): EntryApprovalState { + if (row.voidedAt) return 'Voided'; + if (!row.clockOut || row.status === 'Open') return 'Open'; + if (row.status === 'Approved') return 'Approved'; + if (row.status === 'Rejected') return 'Rejected'; + if (!validManager(row)) return 'BlockedManager'; + return row.frameApprovedRevision === row.revision && + row.frameApproverId === row.employee.managerId + ? 'PendingHr' + : 'PendingManager'; +} + +@Injectable() +export class TimeEntryApprovalsService { + constructor( + private readonly prisma: PrismaService, + private readonly events: EventsGateway, + ) {} + + private async assertTeam(tx: Prisma.TransactionClient): Promise { + const installation = await tx.installationSettings.findUnique({ + where: { id: 1 }, + }); + if (installation?.mode === 'Solo') + throw new ForbiddenException( + 'Team time-entry approvals are disabled in Solo mode', + ); + } + + private async actor( + tx: Prisma.TransactionClient, + id: string, + ): Promise { + const actor = await tx.employee.findUnique({ where: { id } }); + if (!actor?.isActive) + throw new ForbiddenException('Active employee required'); + return actor; + } + + private assertReviewAccess( + row: ReviewEntry, + actor: Employee, + allowOwner: boolean, + ): void { + if (allowOwner && actor.id === row.employeeId) return; + if (actor.id === row.employeeId) + throw new ForbiddenException('You cannot review your own time entry'); + if (actor.role === 'HRAdmin') return; + if (actor.role === 'Manager' && row.employee.managerId === actor.id) return; + throw new ForbiddenException( + 'Only the assigned manager or HR may review this time entry', + ); + } + + private assertApplicable(row: ReviewEntry): void { + if (row.approvalMode === 'Solo' || !row.requiresApproval) + throw new ConflictException( + 'This time entry does not require Team frame approval', + ); + } + + private dto(row: ReviewEntry, actor: Employee): TimeEntryApprovalDto { + const state = frameApprovalState(row); + return { + entry: toTimeEntryDto(row), + revision: row.revision, + employeeName: `${row.employee.firstName} ${row.employee.lastName}`, + managerId: row.employee.managerId, + managerName: row.employee.manager + ? `${row.employee.manager.firstName} ${row.employee.manager.lastName}` + : null, + state, + canDecide: + row.employee.isActive && + actor.id !== row.employeeId && + ((state === 'PendingManager' && + actor.role === 'Manager' && + row.employee.managerId === actor.id) || + (state === 'PendingHr' && + actor.role === 'HRAdmin' && + row.frameApproverId !== actor.id)), + }; + } + + async list( + actorId: string, + scope = 'review', + ): Promise { + if (scope !== 'review' && scope !== 'own') + throw new BadRequestException('Invalid approval scope'); + return this.prisma.$transaction(async (tx) => { + await this.assertTeam(tx); + const actor = await this.actor(tx, actorId); + if ( + scope === 'review' && + actor.role !== 'Manager' && + actor.role !== 'HRAdmin' + ) + throw new ForbiddenException('Manager or HR access required'); + const rows = await tx.timeEntry.findMany({ + where: { + AND: [ + { OR: [{ approvalMode: null }, { approvalMode: { not: 'Solo' } }] }, + { + OR: [ + { requiresApproval: true }, + { auditEvents: { some: { action: { startsWith: 'Frame' } } } }, + ], + }, + ], + ...(scope === 'own' + ? { employeeId: actor.id } + : { + employeeId: { not: actor.id }, + employee: { + isActive: true, + ...(actor.role === 'Manager' ? { managerId: actor.id } : {}), + }, + }), + }, + include: APPROVAL_INCLUDE, + orderBy: [{ clockIn: 'desc' }, { id: 'asc' }], + }); + return rows.map((row) => this.dto(row, actor)); + }); + } + + async history( + id: string, + user: JwtUser, + ): Promise { + return this.prisma.$transaction(async (tx) => { + await this.assertTeam(tx); + const actor = await this.actor(tx, user.id); + const row = await tx.timeEntry.findUnique({ + where: { id }, + include: APPROVAL_INCLUDE, + }); + if (!row) throw new NotFoundException('Time entry not found'); + this.assertReviewAccess(row, actor, true); + // A split may move the original ID wholly inside the frame. Its old + // decisions and invalidation remain reviewable, even though it no + // longer needs approval. Solo archives must remain inaccessible. + if (row.approvalMode === 'Solo') + throw new ForbiddenException( + 'Solo entries have no Team approval history', + ); + const events = await tx.timeEntryAudit.findMany({ + where: { timeEntryId: id, action: { startsWith: 'Frame' } }, + include: { actor: { select: { firstName: true, lastName: true } } }, + orderBy: [{ occurredAt: 'asc' }, { id: 'asc' }], + }); + return events.map((event) => ({ + id: event.id, + actorId: event.actorId, + action: event.action, + reason: event.reason, + actorName: event.actor + ? `${event.actor.firstName} ${event.actor.lastName}` + : null, + occurredAt: event.occurredAt.toISOString(), + })); + }); + } + + async decide( + id: string, + dto: DecideTimeEntryApprovalDto, + user: JwtUser, + ): Promise { + const result = await this.prisma.$transaction(async (tx) => { + // Same serialization order as every booking/correction and installation + // mode change. Row locks also serialize concurrent role/manager changes. + await tx.$executeRaw`SELECT pg_advisory_xact_lock(7261500)`; + await this.assertTeam(tx); + const initial = await tx.timeEntry.findUnique({ where: { id } }); + if (!initial) throw new NotFoundException('Time entry not found'); + await tx.$executeRaw`SELECT pg_advisory_xact_lock(hashtextextended(${initial.employeeId}, 0))`; + await tx.$queryRaw`SELECT "id" FROM "Employee" WHERE "id" = ${initial.employeeId}::uuid FOR SHARE`; + const employee = await tx.employee.findUniqueOrThrow({ + where: { id: initial.employeeId }, + }); + const ids = [ + ...new Set( + [user.id, employee.id, employee.managerId].filter( + (value): value is string => !!value, + ), + ), + ].sort(); + await tx.$queryRaw( + Prisma.sql`SELECT "id" FROM "Employee" WHERE "id" IN (${Prisma.join(ids.map((value) => Prisma.sql`${value}::uuid`))}) ORDER BY "id" FOR SHARE`, + ); + const actor = await this.actor(tx, user.id); + const row = await tx.timeEntry.findUniqueOrThrow({ + where: { id }, + include: APPROVAL_INCLUDE, + }); + this.assertReviewAccess(row, actor, false); + this.assertApplicable(row); + if (!row.employee.isActive) + throw new ForbiddenException('Employee account is inactive'); + if (row.revision !== dto.revision) + throw new ConflictException( + 'Time entry changed; reload before deciding', + ); + const view = this.dto(row, actor); + if ( + (dto.stage === 'Manager' && actor.role !== 'Manager') || + (dto.stage === 'HR' && actor.role !== 'HRAdmin') + ) + throw new ForbiddenException('Wrong approval role'); + if ( + !view.canDecide || + view.state !== + (dto.stage === 'Manager' ? 'PendingManager' : 'PendingHr') + ) + throw new ConflictException( + 'Time entry is not awaiting this approval stage', + ); + const note = dto.note?.trim() || null; + if (dto.outcome === 'Reject' && !note) + throw new BadRequestException('A rejection reason is required'); + const approveManager = + dto.stage === 'Manager' && dto.outcome === 'Approve'; + const data: Prisma.TimeEntryUncheckedUpdateInput = { + revision: { increment: 1 }, + status: + dto.outcome === 'Reject' + ? 'Rejected' + : dto.stage === 'HR' + ? 'Approved' + : 'Pending', + ...(approveManager + ? { + frameApproverId: actor.id, + frameApprovedRevision: row.revision + 1, + } + : {}), + }; + const changed = await tx.timeEntry.updateMany({ + where: { + id, + revision: dto.revision, + status: 'Pending', + clockOut: { not: null }, + voidedAt: null, + }, + data, + }); + if (changed.count !== 1) + throw new ConflictException( + 'Time entry changed; reload before deciding', + ); + const updated = await tx.timeEntry.findUniqueOrThrow({ + where: { id }, + include: APPROVAL_INCLUDE, + }); + const action = + dto.stage === 'Manager' + ? dto.outcome === 'Approve' + ? 'FrameManagerApproved' + : 'FrameManagerRejected' + : dto.outcome === 'Approve' + ? 'FrameHrConfirmed' + : 'FrameHrRejected'; + await tx.timeEntryAudit.create({ + data: { + timeEntryId: id, + actorId: actor.id, + action, + reason: note, + before: { + revision: row.revision, + state: view.state, + status: row.status, + }, + after: { + revision: updated.revision, + state: frameApprovalState(updated), + status: updated.status, + }, + }, + }); + return this.dto(updated, actor); + }); + this.events.broadcast('time-entry:updated', { + id, + employeeId: result.entry.employeeId, + clockOut: result.entry.clockOut, + }); + return result; + } +} diff --git a/apps/web/src/api/client.ts b/apps/web/src/api/client.ts index d47a9e0..9607e03 100644 --- a/apps/web/src/api/client.ts +++ b/apps/web/src/api/client.ts @@ -191,6 +191,41 @@ export interface TimeEntryDto { summary: TimeSummaryDto | null; } +export type TimeEntryApprovalState = + | 'Open' + | 'BlockedManager' + | 'PendingManager' + | 'PendingHr' + | 'Approved' + | 'Rejected' + | 'Voided'; + +export interface TimeEntryApprovalDto { + entry: TimeEntryDto; + revision: number; + employeeName: string; + managerId: string | null; + managerName: string | null; + state: TimeEntryApprovalState; + canDecide: boolean; +} + +export interface TimeEntryApprovalDecision { + revision: number; + stage: 'Manager' | 'HR'; + outcome: 'Approve' | 'Reject'; + note?: string; +} + +export interface TimeEntryApprovalEventDto { + id: string; + actorId: string | null; + actorName: string | null; + action: string; + reason: string | null; + occurredAt: string; +} + export interface SplitTimeEntryResult { first: TimeEntryDto; second: TimeEntryDto; @@ -922,6 +957,19 @@ export const api = { if (to) params.set('to', to); return request(`/api/timeentries?${params.toString()}`); }, + timeEntryApprovals: (scope: 'own' | 'review' = 'review') => + request( + `/api/timeentries/approvals?scope=${scope}`, + ), + decideTimeEntryApproval: (id: string, payload: TimeEntryApprovalDecision) => + request(`/api/timeentries/${id}/approval`, { + method: 'POST', + body: JSON.stringify(payload), + }), + timeEntryApprovalHistory: (id: string) => + request( + `/api/timeentries/${id}/approval-history`, + ), clockIn: (payload: ClockInPayload) => request('/api/timeentries/clock-in', { method: 'POST', diff --git a/apps/web/src/api/generated.ts b/apps/web/src/api/generated.ts index 41d8884..ae14830 100644 --- a/apps/web/src/api/generated.ts +++ b/apps/web/src/api/generated.ts @@ -676,6 +676,54 @@ export interface paths { patch?: never; trace?: never; }; + '/api/timeentries/approvals': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations['TimeEntriesController_approvalList']; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + '/api/timeentries/{id}/approval-history': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: operations['TimeEntriesController_approvalHistory']; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + '/api/timeentries/{id}/approval': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + post: operations['TimeEntriesController_decideApproval']; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; '/api/timeentries/clock-in': { parameters: { query?: never; @@ -2348,6 +2396,42 @@ export interface components { activity: string | null; summary: components['schemas']['TimeSummaryDto'] | null; }; + TimeEntryApprovalDto: { + entry: components['schemas']['TimeEntryDto']; + revision: number; + employeeName: string; + /** Format: uuid */ + managerId: string | null; + managerName: string | null; + /** @enum {string} */ + state: + | 'Open' + | 'BlockedManager' + | 'PendingManager' + | 'PendingHr' + | 'Approved' + | 'Rejected' + | 'Voided'; + canDecide: boolean; + }; + TimeEntryApprovalEventDto: { + id: string; + /** Format: uuid */ + actorId: string | null; + actorName: string | null; + action: string; + reason: string | null; + /** Format: date-time */ + occurredAt: string; + }; + DecideTimeEntryApprovalDto: { + revision: number; + /** @enum {string} */ + stage: 'Manager' | 'HR'; + /** @enum {string} */ + outcome: 'Approve' | 'Reject'; + note?: string; + }; ClockInDto: { note?: string | null; billable?: boolean; @@ -4269,6 +4353,73 @@ export interface operations { }; }; }; + TimeEntriesController_approvalList: { + parameters: { + query?: { + scope?: 'own' | 'review'; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['TimeEntryApprovalDto'][]; + }; + }; + }; + }; + TimeEntriesController_approvalHistory: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + 200: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['TimeEntryApprovalEventDto'][]; + }; + }; + }; + }; + TimeEntriesController_decideApproval: { + parameters: { + query?: never; + header?: never; + path: { + id: string; + }; + cookie?: never; + }; + requestBody: { + content: { + 'application/json': components['schemas']['DecideTimeEntryApprovalDto']; + }; + }; + responses: { + 201: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['TimeEntryApprovalDto']; + }; + }; + }; + }; TimeEntriesController_clockIn: { parameters: { query?: never; diff --git a/apps/web/src/app/i18n.tsx b/apps/web/src/app/i18n.tsx index 2d67e1c..76a2b8a 100644 --- a/apps/web/src/app/i18n.tsx +++ b/apps/web/src/app/i18n.tsx @@ -16,6 +16,69 @@ type Catalog = Record; const STORAGE_KEY = 'openclockwork.locale'; const de: Catalog = { + 'frameApproval.ownTitle': 'Rahmenzeit-Genehmigungen für meine Zeitbuchungen', + 'frameApproval.title': 'Zeitbuchungen', + 'frameApproval.chain': + 'Zeitbuchungen außerhalb der Rahmenzeit benötigen zuerst die Genehmigung der zugewiesenen Führungskraft und danach die Bestätigung durch HR. Beide bearbeiten sie unter Genehmigungen → Zeitbuchungen.', + 'frameApproval.managerChanged': + 'Solange HR noch nicht bestätigt hat, muss nach einer Änderung der zugewiesenen Führungskraft die neue Führungskraft erneut genehmigen.', + 'frameApproval.refresh': 'Genehmigungen aktualisieren', + 'frameApproval.empty': + 'Keine genehmigungspflichtigen Zeitbuchungen vorhanden.', + 'frameApproval.loadFailed': + 'Zeitbuchungs-Genehmigungen konnten nicht geladen werden.', + 'frameApproval.historyFailed': + 'Der Genehmigungsverlauf konnte nicht geladen werden.', + 'frameApproval.decisionFailed': 'Die Entscheidung wurde nicht gespeichert.', + 'frameApproval.refreshHint': + 'Bitte aktualisieren und den aktuellen Stand prüfen, bevor du erneut entscheidest.', + 'frameApproval.notAllowed': 'Diese Entscheidung ist nicht zulässig.', + 'frameApproval.note': 'Notiz (bei Ablehnung erforderlich)', + 'frameApproval.reasonRequired': + 'Für eine Ablehnung ist eine Begründung erforderlich.', + 'frameApproval.assignedManager': 'zugewiesene Führungskraft', + 'frameApproval.history': 'Genehmigungsverlauf', + 'frameApproval.noHistory': + 'Noch keine Entscheidungen oder Änderungen vorhanden.', + 'frameApproval.system': 'System', + 'frameApproval.managerResult': + 'Führungskraft-Genehmigung gespeichert. Als Nächstes bestätigt HR unter Genehmigungen → Zeitbuchungen.', + 'frameApproval.hrResult': + 'HR-Bestätigung gespeichert. Die Zeitbuchung ist genehmigt.', + 'frameApproval.rejectedResult': 'Ablehnung mit Begründung gespeichert.', + 'frameApproval.approve.Manager': 'Als Führungskraft genehmigen', + 'frameApproval.approve.HR': 'Als HR bestätigen', + 'frameApproval.reject.Manager': 'Als Führungskraft ablehnen', + 'frameApproval.reject.HR': 'Als HR ablehnen', + 'frameApproval.state.Open': 'Offen – zuerst ausstempeln', + 'frameApproval.state.BlockedManager': 'Blockiert – Führungskraft fehlt', + 'frameApproval.state.PendingManager': 'Wartet auf Führungskraft', + 'frameApproval.state.PendingHr': 'Wartet auf HR', + 'frameApproval.state.Approved': 'Genehmigt', + 'frameApproval.state.Rejected': 'Abgelehnt', + 'frameApproval.state.Voided': 'Ungültig', + 'frameApproval.help.Open': + 'Bitte zuerst ausstempeln. Erst die abgeschlossene Zeitbuchung kann von der zugewiesenen Führungskraft und danach von HR unter Genehmigungen → Zeitbuchungen geprüft werden.', + 'frameApproval.help.BlockedManager': + 'Bitte HR bitten, eine aktive Führungskraft zuzuweisen. Danach genehmigt diese Führungskraft zuerst, anschließend bestätigt HR unter Genehmigungen → Zeitbuchungen. Es gibt keine direkte HR-Freigabe ohne Führungskraft-Genehmigung.', + 'frameApproval.help.PendingManager': + 'Nächster Schritt: {manager} prüft als zugewiesene Führungskraft unter Genehmigungen → Zeitbuchungen. Danach bestätigt HR am selben Ort.', + 'frameApproval.help.PendingHr': + 'Die Führungskraft hat genehmigt. Nächster Schritt: HR bestätigt unter Genehmigungen → Zeitbuchungen.', + 'frameApproval.help.Approved': + 'Die Zeitbuchung ist genehmigt. Dokumentierte Entscheidungen stehen mit Person und Zeitpunkt im Genehmigungsverlauf.', + 'frameApproval.help.Rejected': + 'Die Zeitbuchung wurde abgelehnt. Begründung und entscheidende Person stehen im Genehmigungsverlauf.', + 'frameApproval.help.Voided': + 'Diese Zeitbuchung ist nicht mehr gültig und kann nicht genehmigt werden. Der Verlauf bleibt einsehbar.', + 'frameApproval.event.FrameSegmentCreated': + 'Buchungssegment nach Änderung neu angelegt', + 'frameApproval.event.FrameManagerApproved': 'Von Führungskraft genehmigt', + 'frameApproval.event.FrameManagerRejected': 'Von Führungskraft abgelehnt', + 'frameApproval.event.FrameHrConfirmed': 'Von HR bestätigt', + 'frameApproval.event.FrameHrRejected': 'Von HR abgelehnt', + 'frameApproval.event.FrameApprovalInvalidated': + 'Bisherige Genehmigung durch Änderung aufgehoben', ...soloDe, ...terminalPinDe, 'projects.deleteFailed': 'Projekt konnte nicht gelöscht werden', @@ -706,6 +769,64 @@ const de: Catalog = { }; const en: Catalog = { + 'frameApproval.ownTitle': 'Approvals for my out-of-frame time entries', + 'frameApproval.title': 'Time entries', + 'frameApproval.chain': + 'Out-of-frame time entries first need approval by the assigned manager, then confirmation by HR. Both review them under Approvals → Time entries.', + 'frameApproval.managerChanged': + 'Before HR has confirmed, changing the assigned manager requires the new manager to approve again.', + 'frameApproval.refresh': 'Refresh approvals', + 'frameApproval.empty': 'No time entries requiring approval.', + 'frameApproval.loadFailed': 'Could not load time entry approvals.', + 'frameApproval.historyFailed': 'Could not load approval history.', + 'frameApproval.decisionFailed': 'The decision was not saved.', + 'frameApproval.refreshHint': + 'Refresh and check the current status before deciding again.', + 'frameApproval.notAllowed': 'This decision is not permitted.', + 'frameApproval.note': 'Note (required for rejection)', + 'frameApproval.reasonRequired': 'A reason is required to reject an entry.', + 'frameApproval.assignedManager': 'assigned manager', + 'frameApproval.history': 'Approval history', + 'frameApproval.noHistory': 'No decisions or changes yet.', + 'frameApproval.system': 'System', + 'frameApproval.managerResult': + 'Manager approval saved. HR confirms next under Approvals → Time entries.', + 'frameApproval.hrResult': + 'HR confirmation saved. The time entry is approved.', + 'frameApproval.rejectedResult': 'Rejection and reason saved.', + 'frameApproval.approve.Manager': 'Approve as manager', + 'frameApproval.approve.HR': 'Confirm as HR', + 'frameApproval.reject.Manager': 'Reject as manager', + 'frameApproval.reject.HR': 'Reject as HR', + 'frameApproval.state.Open': 'Open – clock out first', + 'frameApproval.state.BlockedManager': 'Blocked – no active manager', + 'frameApproval.state.PendingManager': 'Awaiting manager approval', + 'frameApproval.state.PendingHr': 'Awaiting HR confirmation', + 'frameApproval.state.Approved': 'Approved', + 'frameApproval.state.Rejected': 'Rejected', + 'frameApproval.state.Voided': 'Voided', + 'frameApproval.help.Open': + 'Clock out first. Only a closed time entry can be reviewed by the assigned manager and then HR under Approvals → Time entries.', + 'frameApproval.help.BlockedManager': + 'Ask HR to assign an active manager. That manager must approve first, followed by HR confirmation under Approvals → Time entries. HR cannot bypass manager approval.', + 'frameApproval.help.PendingManager': + 'Next: {manager}, the assigned manager, reviews under Approvals → Time entries. HR then confirms in the same place.', + 'frameApproval.help.PendingHr': + 'The manager has approved. Next: HR confirms under Approvals → Time entries.', + 'frameApproval.help.Approved': + 'This time entry is approved. Recorded decisions, reviewers and timestamps are available in approval history.', + 'frameApproval.help.Rejected': + 'The time entry was rejected. See approval history for the reason and reviewer.', + 'frameApproval.help.Voided': + 'This time entry is no longer valid and cannot be approved. Its history remains available.', + 'frameApproval.event.FrameSegmentCreated': + 'Entry segment created after a change', + 'frameApproval.event.FrameManagerApproved': 'Approved by manager', + 'frameApproval.event.FrameManagerRejected': 'Rejected by manager', + 'frameApproval.event.FrameHrConfirmed': 'Confirmed by HR', + 'frameApproval.event.FrameHrRejected': 'Rejected by HR', + 'frameApproval.event.FrameApprovalInvalidated': + 'Previous approval invalidated by a change', ...soloEn, ...terminalPinEn, 'projects.deleteFailed': 'Could not delete project', diff --git a/apps/web/src/app/realtime.ts b/apps/web/src/app/realtime.ts index 0ff985d..ad4971d 100644 --- a/apps/web/src/app/realtime.ts +++ b/apps/web/src/app/realtime.ts @@ -73,6 +73,8 @@ export function useRealtimeInvalidation() { qc.invalidateQueries({ queryKey: ['solo-summary'] }); qc.invalidateQueries({ queryKey: ['solo-hints'] }); qc.invalidateQueries({ queryKey: ['time-entries'] }); + qc.invalidateQueries({ queryKey: ['time-entry-approvals'] }); + qc.invalidateQueries({ queryKey: ['time-entry-approval-history'] }); qc.invalidateQueries({ queryKey: ['account'] }); qc.invalidateQueries({ queryKey: ['violations'] }); }; diff --git a/apps/web/src/components/timeentries/TimeEntryApprovals.spec.tsx b/apps/web/src/components/timeentries/TimeEntryApprovals.spec.tsx new file mode 100644 index 0000000..5487f0e --- /dev/null +++ b/apps/web/src/components/timeentries/TimeEntryApprovals.spec.tsx @@ -0,0 +1,588 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { renderWithProviders } from '../../test-utils'; +import type { TimeEntryApprovalDto, TimeEntryDto } from '../../api/client'; +import { TimeEntryApprovals } from './TimeEntryApprovals'; + +const mocks = vi.hoisted(() => ({ + user: { id: 'manager-1', role: 'Manager' }, + list: vi.fn(), + decide: vi.fn(), + history: vi.fn(), +})); +vi.mock('../../api/client', () => ({ + api: { + timeEntryApprovals: mocks.list, + decideTimeEntryApproval: mocks.decide, + timeEntryApprovalHistory: mocks.history, + }, +})); +vi.mock('../../app/auth', () => ({ useCurrentUser: () => mocks.user })); + +const entry: TimeEntryDto = { + id: 'entry-1', + employeeId: 'employee-1', + clockIn: '2026-05-04T05:00:00Z', + clockOut: '2026-05-04T17:00:00Z', + source: 'web', + status: 'Pending', + requiresApproval: true, + latitude: null, + longitude: null, + accuracyMeters: null, + terminalDistanceMeters: null, + terminalRadiusMeters: null, + terminalMaxAccuracyMeters: null, + positionTimestamp: null, + clockOutLatitude: null, + clockOutLongitude: null, + clockOutAccuracyMeters: null, + clockOutTerminalDistanceMeters: null, + clockOutTerminalRadiusMeters: null, + clockOutTerminalMaxAccuracyMeters: null, + clockOutPositionTimestamp: null, + terminalId: null, + clockOutTerminalId: null, + clockInChallengeId: null, + clockOutChallengeId: null, + projectId: null, + projectCode: null, + projectName: null, + serviceOrderId: null, + serviceOrderNo: null, + serviceOrderTitle: null, + activity: null, + summary: { grossMinutes: 720, breakMinutes: 45, netMinutes: 675 }, +}; +function approval( + overrides: Partial = {}, +): TimeEntryApprovalDto { + return { + entry, + revision: 7, + employeeName: 'Anna Example', + managerId: 'manager-1', + managerName: 'Marc Manager', + state: 'PendingManager', + canDecide: true, + ...overrides, + }; +} +function renderPanel(scope: 'own' | 'review' = 'review') { + return renderWithProviders(); +} + +let storedLocale: string | null; +beforeEach(() => { + storedLocale = 'de'; + vi.spyOn(window, 'localStorage', 'get').mockReturnValue({ + length: 1, + clear: () => { + storedLocale = null; + }, + key: () => 'openclockwork.locale', + removeItem: () => { + storedLocale = null; + }, + getItem: () => storedLocale, + setItem: (_key: string, value: string) => { + storedLocale = value; + }, + }); + mocks.user.id = 'manager-1'; + mocks.user.role = 'Manager'; + mocks.list.mockReset().mockResolvedValue([approval()]); + mocks.decide + .mockReset() + .mockResolvedValue(approval({ state: 'PendingHr', canDecide: false })); + mocks.history.mockReset().mockResolvedValue([]); +}); + +afterEach(() => vi.restoreAllMocks()); + +describe('out-of-frame time entry approvals', () => { + it('shows the manager the correct stage, sends the revision and refetches lists, history and totals', async () => { + const { queryClient } = renderPanel(); + const invalidate = vi.spyOn(queryClient, 'invalidateQueries'); + expect(await screen.findByText('Wartet auf Führungskraft')).toBeDefined(); + expect( + screen.getByText(/Nächster Schritt: Marc Manager/).textContent, + ).toContain('Genehmigungen → Zeitbuchungen'); + expect(mocks.list).toHaveBeenCalledWith('review'); + expect( + screen.queryByRole('button', { name: 'Als HR bestätigen' }), + ).toBeNull(); + fireEvent.click( + screen.getByRole('button', { name: 'Genehmigungsverlauf' }), + ); + await screen.findByText( + 'Noch keine Entscheidungen oder Änderungen vorhanden.', + ); + mocks.list.mockResolvedValue([ + approval({ state: 'PendingHr', canDecide: false }), + ]); + fireEvent.click( + screen.getByRole('button', { name: 'Als Führungskraft genehmigen' }), + ); + await waitFor(() => + expect(mocks.decide).toHaveBeenCalledWith('entry-1', { + revision: 7, + stage: 'Manager', + outcome: 'Approve', + }), + ); + expect(await screen.findByText('Wartet auf HR')).toBeDefined(); + expect(screen.getByRole('status').textContent).toContain( + 'Als Nächstes bestätigt HR', + ); + expect( + screen.queryByRole('button', { name: 'Als Führungskraft genehmigen' }), + ).toBeNull(); + expect(mocks.list.mock.calls.length).toBeGreaterThan(1); + expect(mocks.history.mock.calls.length).toBeGreaterThan(1); + for (const key of [ + 'time-entry-approvals', + 'time-entry-approval-history', + 'time-entries', + 'account', + 'violations', + 'working-time-report', + 'project-report', + ]) { + expect(invalidate).toHaveBeenCalledWith({ queryKey: [key] }); + } + }); + + it.each([ + { + role: 'Manager', + id: 'manager-1', + state: 'PendingManager' as const, + stage: 'Manager', + label: 'Als Führungskraft ablehnen', + }, + { + role: 'HRAdmin', + id: 'hr-1', + state: 'PendingHr' as const, + stage: 'HR', + label: 'Als HR ablehnen', + }, + ])( + 'requires a nonblank reason when $role rejects', + async ({ role, id, state, stage, label }) => { + mocks.user.role = role; + mocks.user.id = id; + mocks.list.mockResolvedValue([approval({ state })]); + renderPanel(); + const reject = await screen.findByRole('button', { name: label }); + expect(reject.hasAttribute('disabled')).toBe(true); + const note = screen.getByLabelText('Notiz (bei Ablehnung erforderlich)'); + fireEvent.change(note, { target: { value: ' ' } }); + expect(reject.hasAttribute('disabled')).toBe(true); + fireEvent.click(reject); + expect(mocks.decide).not.toHaveBeenCalled(); + fireEvent.change(note, { + target: { value: ' Bitte Zeitraum prüfen ' }, + }); + mocks.list.mockResolvedValue([ + approval({ state: 'Rejected', canDecide: false }), + ]); + fireEvent.click(reject); + await waitFor(() => + expect(mocks.decide).toHaveBeenCalledWith('entry-1', { + revision: 7, + stage, + outcome: 'Reject', + note: 'Bitte Zeitraum prüfen', + }), + ); + expect( + await screen.findByText('Ablehnung mit Begründung gespeichert.'), + ).toBeDefined(); + }, + ); + + it('lets HR confirm only the HR stage with the current revision', async () => { + mocks.user.role = 'HRAdmin'; + mocks.user.id = 'hr-1'; + mocks.list.mockResolvedValue([ + approval({ state: 'PendingHr', revision: 12 }), + ]); + renderPanel(); + const confirm = await screen.findByRole('button', { + name: 'Als HR bestätigen', + }); + expect( + screen.queryByRole('button', { name: 'Als Führungskraft genehmigen' }), + ).toBeNull(); + fireEvent.change( + screen.getByLabelText('Notiz (bei Ablehnung erforderlich)'), + { target: { value: 'Geprüft' } }, + ); + mocks.list.mockResolvedValue([ + approval({ state: 'Approved', canDecide: false }), + ]); + fireEvent.click(confirm); + await waitFor(() => + expect(mocks.decide).toHaveBeenCalledWith('entry-1', { + revision: 12, + stage: 'HR', + outcome: 'Approve', + note: 'Geprüft', + }), + ); + expect(await screen.findByText('Genehmigt')).toBeDefined(); + }); + + it('does not let HR bypass the assigned manager', async () => { + mocks.user.role = 'HRAdmin'; + mocks.user.id = 'hr-1'; + renderPanel(); + await screen.findByText('Wartet auf Führungskraft'); + expect( + screen.queryByLabelText('Notiz (bei Ablehnung erforderlich)'), + ).toBeNull(); + expect( + screen.queryByRole('button', { name: 'Als HR bestätigen' }), + ).toBeNull(); + }); + + it.each([ + 'Open', + 'BlockedManager', + 'Approved', + 'Rejected', + 'Voided', + 'PendingHr', + ] as const)( + 'shows %s without manager actions even with an inconsistent canDecide flag', + async (state) => { + mocks.list.mockResolvedValue([approval({ state })]); + renderPanel(); + await screen.findByText(/Anna Example/); + expect( + screen.queryByLabelText('Notiz (bei Ablehnung erforderlich)'), + ).toBeNull(); + expect( + screen.getByRole('button', { name: 'Genehmigungsverlauf' }), + ).toBeDefined(); + }, + ); + + it('respects server permissions and manager assignment', async () => { + mocks.list.mockResolvedValue([ + approval({ canDecide: false }), + approval({ + entry: { ...entry, id: 'entry-2' }, + managerId: 'other-manager', + }), + ]); + renderPanel(); + await screen.findAllByText('Wartet auf Führungskraft'); + expect( + screen.queryByLabelText('Notiz (bei Ablehnung erforderlich)'), + ).toBeNull(); + }); + + it.each(['Employee', 'Manager', 'HRAdmin'])( + 'shows own status/history but no decisions for an owner with role %s', + async (role) => { + mocks.user.role = role; + mocks.user.id = entry.employeeId; + mocks.list.mockResolvedValue([approval({ managerId: entry.employeeId })]); + renderPanel('own'); + await screen.findByText('Wartet auf Führungskraft'); + expect( + screen.getByText('Rahmenzeit-Genehmigungen für meine Zeitbuchungen'), + ).toBeDefined(); + expect(mocks.list).toHaveBeenCalledWith('own'); + expect( + screen.queryByLabelText('Notiz (bei Ablehnung erforderlich)'), + ).toBeNull(); + fireEvent.click( + screen.getByRole('button', { name: 'Genehmigungsverlauf' }), + ); + expect( + await screen.findByText( + 'Noch keine Entscheidungen oder Änderungen vorhanden.', + ), + ).toBeDefined(); + }, + ); + + it('never offers manager-stage actions to HR even with an inconsistent server flag and assignment', async () => { + mocks.user.role = 'HRAdmin'; + mocks.list.mockResolvedValue([ + approval({ managerId: mocks.user.id, canDecide: true }), + ]); + renderPanel(); + await screen.findByText('Wartet auf Führungskraft'); + expect( + screen.queryByLabelText('Notiz (bei Ablehnung erforderlich)'), + ).toBeNull(); + }); + + it('prevents self-approval even in a broad reviewer role', async () => { + mocks.user.role = 'HRAdmin'; + mocks.user.id = entry.employeeId; + mocks.list.mockResolvedValue([approval({ state: 'PendingHr' })]); + renderPanel(); + await screen.findByText('Wartet auf HR'); + expect( + screen.queryByLabelText('Notiz (bei Ablehnung erforderlich)'), + ).toBeNull(); + }); + + it('does not query the reviewer list for an employee', async () => { + mocks.user.role = 'Employee'; + renderPanel(); + expect(mocks.list).not.toHaveBeenCalled(); + expect(screen.queryByText('Zeitbuchungen')).toBeNull(); + }); + + it('keeps stale errors and the entered reason visible, then uses the refreshed revision', async () => { + mocks.decide.mockRejectedValueOnce( + new Error('Stale revision: entry changed'), + ); + renderPanel(); + const reject = await screen.findByRole('button', { + name: 'Als Führungskraft ablehnen', + }); + const note = screen.getByLabelText( + 'Notiz (bei Ablehnung erforderlich)', + ) as HTMLInputElement; + fireEvent.change(note, { target: { value: 'Zeitraum falsch' } }); + fireEvent.click(reject); + expect(await screen.findByRole('alert')).toBeDefined(); + expect(screen.getByRole('alert').textContent).toContain( + 'Stale revision: entry changed', + ); + expect(note.value).toBe('Zeitraum falsch'); + mocks.list.mockResolvedValue([approval({ revision: 9 })]); + fireEvent.click( + screen.getByRole('button', { name: 'Genehmigungen aktualisieren' }), + ); + await waitFor(() => expect(mocks.list).toHaveBeenCalledTimes(2)); + await waitFor(() => + expect( + screen + .getByRole('button', { name: 'Genehmigungen aktualisieren' }) + .hasAttribute('disabled'), + ).toBe(false), + ); + expect(screen.getByRole('alert').textContent).toContain( + 'Stale revision: entry changed', + ); + mocks.decide.mockResolvedValueOnce( + approval({ state: 'Rejected', canDecide: false }), + ); + fireEvent.click( + screen.getByRole('button', { name: 'Als Führungskraft ablehnen' }), + ); + await waitFor(() => + expect(mocks.decide).toHaveBeenLastCalledWith('entry-1', { + revision: 9, + stage: 'Manager', + outcome: 'Reject', + note: 'Zeitraum falsch', + }), + ); + }); + + it('renders every decision and invalidation with actor, timestamp and reason', async () => { + mocks.history.mockResolvedValue([ + { + id: '1', + actorId: 'manager-1', + actorName: 'Marc Manager', + action: 'FrameManagerApproved', + reason: 'Kundentermin', + occurredAt: '2026-05-04T17:30:00Z', + }, + { + id: '2', + actorId: 'hr-1', + actorName: 'Hannah HR', + action: 'FrameHrConfirmed', + reason: null, + occurredAt: '2026-05-04T18:00:00Z', + }, + { + id: '3', + actorId: null, + actorName: null, + action: 'FrameApprovalInvalidated', + reason: 'Führungskraft geändert', + occurredAt: '2026-05-05T08:00:00Z', + }, + { + id: '4', + actorId: 'manager-2', + actorName: 'New Manager', + action: 'FrameManagerRejected', + reason: 'Unklarer Zeitraum', + occurredAt: '2026-05-05T09:00:00Z', + }, + { + id: '5', + actorId: 'hr-1', + actorName: 'Hannah HR', + action: 'FrameHrRejected', + reason: 'Fehlender Nachweis', + occurredAt: '2026-05-05T10:00:00Z', + }, + { + id: '6', + actorId: 'owner-1', + actorName: 'Anna Example', + action: 'FrameSegmentCreated', + reason: null, + occurredAt: '2026-05-05T11:00:00Z', + }, + ]); + const { container } = renderPanel('own'); + fireEvent.click( + await screen.findByRole('button', { name: 'Genehmigungsverlauf' }), + ); + expect(await screen.findByText('Kundentermin')).toBeDefined(); + for (const label of [ + 'Von Führungskraft genehmigt', + 'Von HR bestätigt', + 'Bisherige Genehmigung durch Änderung aufgehoben', + 'Von Führungskraft abgelehnt', + 'Von HR abgelehnt', + 'Buchungssegment nach Änderung neu angelegt', + 'Führungskraft geändert', + 'Unklarer Zeitraum', + 'Fehlender Nachweis', + ]) { + expect(screen.getByText(label)).toBeDefined(); + } + expect(screen.getByText(/Marc Manager ·/)).toBeDefined(); + expect(screen.getByText(/System ·/)).toBeDefined(); + expect(container.querySelector('time')?.getAttribute('datetime')).toBe( + '2026-05-04T17:30:00Z', + ); + }); + + it('shows loading, query errors and recoverable empty states', async () => { + let rejectList!: (reason: Error) => void; + mocks.list.mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectList = reject; + }), + ); + renderPanel(); + expect(screen.getByRole('status').textContent).toBe('Lädt …'); + rejectList(new Error('Service unavailable')); + expect((await screen.findByRole('alert')).textContent).toContain( + 'Service unavailable', + ); + mocks.list.mockResolvedValue([]); + fireEvent.click( + screen.getByRole('button', { name: 'Genehmigungen aktualisieren' }), + ); + expect( + await screen.findByText( + 'Keine genehmigungspflichtigen Zeitbuchungen vorhanden.', + ), + ).toBeDefined(); + }); + + it('surfaces history errors and allows retry', async () => { + mocks.history.mockRejectedValueOnce(new Error('History unavailable')); + renderPanel('own'); + fireEvent.click( + await screen.findByRole('button', { name: 'Genehmigungsverlauf' }), + ); + expect((await screen.findByRole('alert')).textContent).toContain( + 'History unavailable', + ); + mocks.history.mockResolvedValue([]); + fireEvent.click( + screen.getAllByRole('button', { name: 'Genehmigungen aktualisieren' })[1], + ); + expect( + await screen.findByText( + 'Noch keine Entscheidungen oder Änderungen vorhanden.', + ), + ).toBeDefined(); + }); + + it.each([ + { + locale: 'de', + title: 'Zeitbuchungen', + state: 'Blockiert – Führungskraft fehlt', + manager: 'aktive Führungskraft zuzuweisen', + where: 'Genehmigungen → Zeitbuchungen', + }, + { + locale: 'en', + title: 'Time entries', + state: 'Blocked – no active manager', + manager: 'assign an active manager', + where: 'Approvals → Time entries', + }, + ])( + 'explains the chain and missing-manager recovery in $locale', + async ({ locale, title, state, manager, where }) => { + storedLocale = locale; + mocks.list.mockResolvedValue([ + approval({ + state: 'BlockedManager', + managerId: null, + managerName: null, + canDecide: false, + }), + ]); + renderPanel(); + expect(await screen.findByText(state)).toBeDefined(); + expect(screen.getByText(title)).toBeDefined(); + const help = screen.getByText(new RegExp(manager)); + expect(help.textContent).toContain(where); + expect(screen.queryByLabelText(/Note|Notiz/)).toBeNull(); + }, + ); + + it('provides English actions, error guidance and history labels', async () => { + storedLocale = 'en'; + mocks.user.role = 'HRAdmin'; + mocks.user.id = 'hr-1'; + mocks.list.mockResolvedValue([approval({ state: 'PendingHr' })]); + mocks.decide.mockRejectedValueOnce(new Error('Revision conflict')); + mocks.history.mockResolvedValue([ + { + id: '1', + actorId: 'manager-1', + actorName: 'Marc Manager', + action: 'FrameManagerApproved', + reason: 'Client visit', + occurredAt: '2026-05-04T17:30:00Z', + }, + ]); + renderPanel(); + fireEvent.click( + await screen.findByRole('button', { name: 'Confirm as HR' }), + ); + expect((await screen.findByRole('alert')).textContent).toContain( + 'The decision was not saved.', + ); + expect(screen.getByRole('alert').textContent).toContain( + 'Refresh and check the current status', + ); + expect( + screen.getByLabelText('Note (required for rejection)'), + ).toBeDefined(); + expect( + screen + .getByRole('button', { name: 'Reject as HR' }) + .hasAttribute('disabled'), + ).toBe(true); + fireEvent.click(screen.getByRole('button', { name: 'Approval history' })); + expect(await screen.findByText('Approved by manager')).toBeDefined(); + expect(screen.getByText('Client visit')).toBeDefined(); + }); +}); diff --git a/apps/web/src/components/timeentries/TimeEntryApprovals.tsx b/apps/web/src/components/timeentries/TimeEntryApprovals.tsx new file mode 100644 index 0000000..9b20dcf --- /dev/null +++ b/apps/web/src/components/timeentries/TimeEntryApprovals.tsx @@ -0,0 +1,319 @@ +import { useState } from 'react'; +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { Alert, AlertDescription } from '@/components/ui/alert'; +import { Badge } from '@/components/ui/badge'; +import { Button } from '@/components/ui/button'; +import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'; +import { Input } from '@/components/ui/input'; +import { Label } from '@/components/ui/label'; +import { + api, + type TimeEntryApprovalDecision, + type TimeEntryApprovalDto, +} from '../../api/client'; +import { useCurrentUser } from '../../app/auth'; +import { useI18n } from '../../app/i18n'; + +/** All lists and downstream totals must reflect decisions and edited entries. */ +export async function refreshTimeEntryApprovals( + qc: ReturnType, +) { + await Promise.all( + [ + 'time-entry-approvals', + 'time-entry-approval-history', + 'time-entries', + 'account', + 'violations', + 'working-time-report', + 'project-report', + ].map((key) => qc.invalidateQueries({ queryKey: [key] })), + ); +} + +export function TimeEntryApprovals({ scope }: { scope: 'own' | 'review' }) { + const user = useCurrentUser(); + const { t } = useI18n(); + const allowed = + scope === 'own' || user.role === 'Manager' || user.role === 'HRAdmin'; + const query = useQuery({ + queryKey: ['time-entry-approvals', scope, user.id, user.role], + queryFn: () => api.timeEntryApprovals(scope), + enabled: allowed, + }); + if (!allowed) return null; + + return ( + + + + {t( + scope === 'own' ? 'frameApproval.ownTitle' : 'frameApproval.title', + )} + +

+ {t('frameApproval.chain')} +

+

+ {t('frameApproval.managerChanged')} +

+ + + {query.isPending &&

{t('common.loading')}

} + {query.isError && ( + + + {t('frameApproval.loadFailed')}{' '} + {query.error instanceof Error ? query.error.message : ''} + + + )} + + {query.isSuccess && query.data.length === 0 && ( +

+ {t('frameApproval.empty')} +

+ )} + {query.data && query.data.length > 0 && ( +
    + {query.data.map((approval) => ( + + ))} +
+ )} + + + ); +} + +function TimeEntryApprovalRow({ + approval, + scope, +}: { + approval: TimeEntryApprovalDto; + scope: 'own' | 'review'; +}) { + const user = useCurrentUser(); + const { t, formatDateTime } = useI18n(); + const qc = useQueryClient(); + const [note, setNote] = useState(''); + const [historyOpen, setHistoryOpen] = useState(false); + const [result, setResult] = useState(null); + const { entry, state } = approval; + const stage = + state === 'PendingManager' + ? 'Manager' + : state === 'PendingHr' + ? 'HR' + : null; + // canDecide is server-authoritative; also guard role, stage and self-approval. + const canDecide = + scope === 'review' && + approval.canDecide && + entry.employeeId !== user.id && + !!entry.clockOut && + ((stage === 'Manager' && + user.role === 'Manager' && + approval.managerId === user.id) || + (stage === 'HR' && user.role === 'HRAdmin')); + const decision = useMutation({ + mutationFn: (outcome: TimeEntryApprovalDecision['outcome']) => { + // Do not allow a stale UI event to bypass the currently displayed stage. + if (!canDecide || !stage) throw new Error(t('frameApproval.notAllowed')); + if (outcome === 'Reject' && !note.trim()) { + throw new Error(t('frameApproval.reasonRequired')); + } + return api.decideTimeEntryApproval(entry.id, { + revision: approval.revision, + stage, + outcome, + ...(note.trim() ? { note: note.trim() } : {}), + }); + }, + onSuccess: async (_updated, outcome) => { + setResult( + t( + outcome === 'Reject' + ? 'frameApproval.rejectedResult' + : stage === 'Manager' + ? 'frameApproval.managerResult' + : 'frameApproval.hrResult', + ), + ); + setNote(''); + await refreshTimeEntryApprovals(qc); + }, + }); + const labelId = `frame-approval-${entry.id}`; + + return ( +
  • +
    +
    +

    + {approval.employeeName} · {formatDateTime(entry.clockIn)} –{' '} + {entry.clockOut + ? formatDateTime(entry.clockOut) + : t('booking.open')} +

    + {entry.summary && ( +

    + {t('booking.summary', { + gross: `${entry.summary.grossMinutes}min`, + break: entry.summary.breakMinutes, + net: `${entry.summary.netMinutes}min`, + })} +

    + )} + {entry.projectCode && ( +

    + {entry.projectCode} + {entry.serviceOrderNo ? ` · ${entry.serviceOrderNo}` : ''} +

    + )} + {entry.activity &&

    {entry.activity}

    } +
    + + {t(`frameApproval.state.${state}`)} + +
    +

    + {t(`frameApproval.help.${state}`, { + manager: approval.managerName ?? t('frameApproval.assignedManager'), + })} +

    + {result && ( +

    + {result} +

    + )} + {decision.isError && ( + + + {t('frameApproval.decisionFailed')}{' '} + {decision.error instanceof Error ? decision.error.message : ''}{' '} + {t('frameApproval.refreshHint')} + + + )} + {canDecide && ( +
    + + setNote(event.target.value)} + /> +

    + {t('frameApproval.reasonRequired')} +

    +
    + + +
    +
    + )} + + {historyOpen && } +
    • + ); +} + +function ApprovalHistory({ entryId }: { entryId: string }) { + const user = useCurrentUser(); + const { t, formatDateTime } = useI18n(); + const history = useQuery({ + queryKey: ['time-entry-approval-history', entryId, user.id], + queryFn: () => api.timeEntryApprovalHistory(entryId), + }); + return ( +
    + {history.isPending &&

    {t('common.loading')}

    } + {history.isError && ( + + + {t('frameApproval.historyFailed')}{' '} + {history.error instanceof Error ? history.error.message : ''} + + + )} + {history.isSuccess && history.data.length === 0 && ( +

    {t('frameApproval.noHistory')}

    + )} + {history.data && ( +
      + {history.data.map((event) => ( +
    1. +

      + {t(`frameApproval.event.${event.action}`)} +

      +

      + {event.actorName ?? event.actorId ?? t('frameApproval.system')}{' '} + ·{' '} + +

      + {event.reason && ( +

      {event.reason}

      + )} +
      2. + ))} +
    + )} + {history.isError && ( + + )} +
    + ); +} diff --git a/apps/web/src/routes/AdminRequestsPage.spec.tsx b/apps/web/src/routes/AdminRequestsPage.spec.tsx index 30274bf..ba0ae87 100644 --- a/apps/web/src/routes/AdminRequestsPage.spec.tsx +++ b/apps/web/src/routes/AdminRequestsPage.spec.tsx @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { fireEvent, screen } from '@testing-library/react'; import { renderWithProviders } from '../test-utils'; import { AdminRequestsPage } from './AdminRequestsPage'; +import { api } from '../api/client'; const request = { id: 'request-42', @@ -17,6 +18,7 @@ const request = { }; vi.mock('../api/client', () => ({ api: { + timeEntryApprovals: vi.fn().mockResolvedValue([]), listRequests: vi .fn() .mockImplementation(async (filter) => @@ -43,6 +45,8 @@ describe('request identity in approval context (#42)', () => { it('shows the employee name in the inbox and retained audit trail', async () => { renderWithProviders(); expect(await screen.findByText(/Anna Example · Urlaub/)).toBeDefined(); + expect(await screen.findByText('Zeitbuchungen')).toBeDefined(); + expect(api.timeEntryApprovals).toHaveBeenCalledWith('review'); fireEvent.click(screen.getByRole('button', { name: 'Verlauf' })); expect( await screen.findByText(/Anna Example · Urlaub · 4.5.2026/), diff --git a/apps/web/src/routes/AdminRequestsPage.tsx b/apps/web/src/routes/AdminRequestsPage.tsx index e7ef36f..819cc25 100644 --- a/apps/web/src/routes/AdminRequestsPage.tsx +++ b/apps/web/src/routes/AdminRequestsPage.tsx @@ -25,6 +25,7 @@ import { api, type BulkResult, type RequestDto } from '../api/client'; import { useCurrentUser } from '../app/auth'; import { cn } from '@/lib/utils'; import { useI18n } from '../app/i18n'; +import { TimeEntryApprovals } from '../components/timeentries/TimeEntryApprovals'; type BulkDialog = | { mode: 'approve'; ids: string[] } @@ -107,6 +108,8 @@ export function AdminRequestsPage() {

    + + {bulkOutcome && ( ({ api: { timeEntries: (...args: unknown[]) => timeEntriesMock(...args), + timeEntryApprovals: (...args: unknown[]) => timeEntryApprovalsMock(...args), + timeEntryApprovalHistory: (...args: unknown[]) => + approvalHistoryMock(...args), clockIn: (...args: unknown[]) => clockInMock(...args), clockOut: (...args: unknown[]) => clockOutMock(...args), dailyBlockOption: (...args: unknown[]) => dailyBlockOptionMock(...args), @@ -89,6 +94,8 @@ const PROJECTS = [ describe('BookingPage', () => { beforeEach(() => { timeEntriesMock.mockClear().mockResolvedValue([]); + timeEntryApprovalsMock.mockClear().mockResolvedValue([]); + approvalHistoryMock.mockClear().mockResolvedValue([]); clockInMock .mockClear() .mockResolvedValue(entryFixture({ clockOut: null, status: 'Open' })); @@ -113,6 +120,49 @@ describe('BookingPage', () => { onlineState = true; }); + it('makes own approval status, next reviewer and history discoverable beside bookings', async () => { + timeEntryApprovalsMock.mockResolvedValue([ + { + entry: entryFixture({ requiresApproval: true }), + revision: 4, + employeeName: 'Test User', + managerId: 'manager-1', + managerName: 'Maria Manager', + state: 'PendingManager', + canDecide: false, + }, + ]); + approvalHistoryMock.mockResolvedValue([ + { + id: 'history-1', + actorId: 'manager-1', + actorName: 'Maria Manager', + action: 'FrameApprovalInvalidated', + reason: 'Zeitraum geändert', + occurredAt: '2026-05-04T16:00:00Z', + }, + ]); + const { BookingPage } = await import('./BookingPage'); + renderWithProviders(); + expect( + await screen.findByText( + 'Rahmenzeit-Genehmigungen für meine Zeitbuchungen', + ), + ).toBeDefined(); + expect(await screen.findByText('Wartet auf Führungskraft')).toBeDefined(); + expect( + screen.getByText(/Nächster Schritt: Maria Manager/).textContent, + ).toContain('Genehmigungen → Zeitbuchungen'); + expect(timeEntryApprovalsMock).toHaveBeenCalledWith('own'); + expect( + screen.queryByRole('button', { name: 'Als Führungskraft genehmigen' }), + ).toBeNull(); + fireEvent.click( + screen.getByRole('button', { name: 'Genehmigungsverlauf' }), + ); + expect(await screen.findByText('Zeitraum geändert')).toBeDefined(); + }); + it('renders the "not clocked in" state when there is no open entry', async () => { const { BookingPage } = await import('./BookingPage'); renderWithProviders(); diff --git a/apps/web/src/routes/BookingPage.tsx b/apps/web/src/routes/BookingPage.tsx index c7102e1..ad17be8 100644 --- a/apps/web/src/routes/BookingPage.tsx +++ b/apps/web/src/routes/BookingPage.tsx @@ -30,6 +30,10 @@ import { import { useCurrentUser } from '../app/auth'; import { useOnline } from '../app/use-online'; import { useI18n } from '../app/i18n'; +import { + refreshTimeEntryApprovals, + TimeEntryApprovals, +} from '../components/timeentries/TimeEntryApprovals'; function captureGps(): Promise<{ latitude: number; @@ -342,7 +346,7 @@ export function BookingPage() { if (context?.previous) qc.setQueryData(entriesKey, context.previous); setError(e instanceof Error ? e.message : t('booking.failed')); }, - onSettled: () => qc.invalidateQueries({ queryKey: entriesKey }), + onSettled: () => refreshTimeEntryApprovals(qc), }); const clockOutMutation = useMutation< @@ -365,7 +369,7 @@ export function BookingPage() { if (context?.previous) qc.setQueryData(entriesKey, context.previous); setError(e instanceof Error ? e.message : t('booking.clockOutFailed')); }, - onSettled: () => qc.invalidateQueries({ queryKey: entriesKey }), + onSettled: () => refreshTimeEntryApprovals(qc), }); return ( @@ -530,6 +534,8 @@ export function BookingPage() { + + {t('booking.lastEntries')} @@ -589,11 +595,12 @@ export function BookingPage() { {t('booking.dailyBlockSource')} )} - {e.requiresApproval && ( - - {t('booking.approval')} - - )} + {e.requiresApproval && + (e.status === 'Pending' || e.status === 'Open') && ( + + {t('booking.approval')} + + )} setEditDialogEntry(null)} onSaved={() => { setEditDialogEntry(null); - qc.invalidateQueries({ queryKey: entriesKey }); + void refreshTimeEntryApprovals(qc); }} /> )} @@ -651,7 +658,7 @@ export function BookingPage() { onClose={() => setSplitDialogEntry(null)} onSaved={() => { setSplitDialogEntry(null); - qc.invalidateQueries({ queryKey: entriesKey }); + void refreshTimeEntryApprovals(qc); }} /> )} @@ -662,7 +669,7 @@ export function BookingPage() { onClose={() => setBookRangeOpen(false)} onSaved={() => { setBookRangeOpen(false); - qc.invalidateQueries({ queryKey: entriesKey }); + void refreshTimeEntryApprovals(qc); }} /> )} @@ -673,9 +680,7 @@ export function BookingPage() { onClose={() => setDailyBlockOpen(false)} onSaved={() => { setDailyBlockOpen(false); - qc.invalidateQueries({ queryKey: entriesKey }); - qc.invalidateQueries({ queryKey: ['account', employeeId] }); - qc.invalidateQueries({ queryKey: ['violations', employeeId] }); + void refreshTimeEntryApprovals(qc); }} /> )} diff --git a/docs/ISSUE_FIXES_MANUAL_TEST.de.md b/docs/ISSUE_FIXES_MANUAL_TEST.de.md index 073b92d..8a66cb0 100644 --- a/docs/ISSUE_FIXES_MANUAL_TEST.de.md +++ b/docs/ISSUE_FIXES_MANUAL_TEST.de.md @@ -300,5 +300,239 @@ TEST_WEB_PORT=18042 docker compose --env-file /dev/null \ ``` Zum Fortsetzen denselben `up -d --wait`-Befehl verwenden, **nicht** erneut seeden. -Kein `down -v`: Das würde die Nachtestdaten löschen. Folgepakete #41, #43, #45 -und #44 ergänzen diese Anleitung erst bei ihrer jeweiligen Implementierung. +Kein `down -v`: Das würde die Nachtestdaten löschen. Weitere Pakete ergänzen +diese Anleitung erst bei ihrer jeweiligen Implementierung. + +## Paket #41 – Freigabe normaler Buchungen außerhalb der Rahmenzeit + +### Branch, Abhängigkeit und isoliertes Setup + +- Branch: `fix/41-outside-frame-approval`. +- Worktree: `/Users/patrickschiller/openclockwork-issue-41`. +- Basis ist der akzeptierte #42-Head `4ff247bc18adfc0b245d8dbb044f7968cf2a4678`. +- Der PR zielt für die vorhandene vollständige CI auf `main`, enthält deshalb + **kumulativ #42 + #41** und hängt von PR #46 ab. Vorgesehene Reihenfolge: + zuerst #46, danach #41; hier wird nichts gemergt. Der aktuelle obere Branch + kann ohne Merge bereits beide Pakete gemeinsam testen. +- Additive Migration `20261107031000_time_entry_frame_approval`: zwei nullable + Signaturfelder. Bestehende Buchungswerte, Revisionen und Historien bleiben + erhalten; alte ausstehende Buchungen beginnen bei der Managerprüfung. + +Grundlage weiterhin [LOCAL_DOCKER_TEST.de.md](LOCAL_DOCKER_TEST.de.md). +Einen separaten Stack verwenden; keine bestehenden Projekte/Volumes löschen. +Im #41-Worktree oder einem neuen Checkout dieses Branches: + +```bash +git fetch origin +git worktree add ../openclockwork-41-manual origin/fix/41-outside-frame-approval +cd ../openclockwork-41-manual + +cat > /tmp/openclockwork-41-compose.yml <<'YAML' +services: + api: + image: openclockwork-api:issue41-local + web: + image: openclockwork-web:issue41-local +YAML + +TEST_WEB_PORT=18041 docker compose --env-file /dev/null \ + -p openclockwork-issue41 -f docker-compose.test.yml \ + -f /tmp/openclockwork-41-compose.yml build + +TEST_WEB_PORT=18041 docker compose --env-file /dev/null \ + -p openclockwork-issue41 -f docker-compose.test.yml \ + -f /tmp/openclockwork-41-compose.yml up -d --wait + +TEST_WEB_PORT=18041 docker compose --env-file /dev/null \ + -p openclockwork-issue41 -f docker-compose.test.yml \ + -f /tmp/openclockwork-41-compose.yml exec -T \ + -e NODE_ENV=development \ + -e OPENCLOCKWORK_SEED_CONFIRM_DATABASE=openclockwork_test \ + api node --import tsx prisma/seed.ts +``` + +Seed nur auf der **neuen leeren Datenbank** ausführen, beim Wiederaufnehmen +nicht erneut. Oberfläche: . API-Zeitzone im bestehenden +Test-Compose ist Europe/Berlin. Die synthetischen Rollen und das lokale Passwort +stehen im #42-Setup oben: Anna = Beschäftigte, Marc = zugewiesener Manager, +Hannah = HR, Bernd = anderer Beschäftigter. Separate Browserprofile verwenden. +Anna muss einem aktiven Manager mit Rolle **Manager** zugeordnet sein. +Rolle HRAdmin ersetzt diese erste Stufe ausdrücklich nicht. + +### A. Tatsächliches Stempeln und Auffindbarkeit + +1. Als Hannah unter Mitarbeitenden/Arbeitszeitplänen für Anna einen Plan wählen, + dessen Rahmen die **aktuelle Server-Ortszeit nicht einschließt** (z. B. + 07:00–08:00, wenn gerade nachmittags getestet wird). Vorherige Zuordnung + notieren. Keine laufende Session und keinen Tagesblock für den Testtag nutzen. +2. Als Anna **Buchen → Kommen**, kurz darauf **Gehen**. Es muss ein positives, + abgeschlossenes Intervall entstehen. Während die Uhr läuft: kein Entscheiden + möglich, auch nicht per direktem Freigabe-API-Aufruf. +3. In **Buchen** ist der eigene Freigabebereich sichtbar. Er benennt die + Managerprüfung, Marc und den Ort **Genehmigungen → Zeitbuchungen** sowie + die danach zwingende HR-Bestätigung. Es handelt sich um eine normale + PWA-Zeitbuchung, nicht um einen neu anzulegenden Zeitkorrekturantrag. +4. Als Marc **Genehmigungen** öffnen: Im Bereich für Zeitbuchungen steht Anna + mit Zeitraum und Status. Managerfreigabe ausführen. Erwartet: + `PendingHr`, weiterhin Buchungsstatus `Pending`; noch kein HR-Endergebnis. +5. Als Hannah dieselbe Seite öffnen, HR bestätigen. Erwartet: dieselbe + Buchungs-ID, `Approved`; Anna und Marc sehen nach Aktualisierung das Ergebnis. + Der Verlauf zeigt beide tatsächlichen Akteure, Zeitpunkte, Ergebnisse und + gegebenenfalls Notizen. Es entsteht kein zusätzlicher Antrag. +6. Auf Englisch wiederholen. Alle neuen Stufen, Hilfen, Aktionen, Fehler und + Historienereignisse sind übersetzt; mit Tab/Enter bedienbar. + +### B. Reproduzierbare Legacy-Ausgangsbuchungen + +Zum gezielten Nachtest alter ausstehender Einträge können auf **diesem neuen +synthetischen Stack** drei feste Intervalle angelegt werden. Die Seed-Zeiten +liegen relativ zum aktuellen Datum; vorab prüfen, dass die folgenden historischen +Oktobertage frei sind. Bei Konflikt keine vorhandenen Daten ändern, sondern +freie Tage im SQL wählen. Die neuen Spalten absichtlich nicht setzen: +Das bildet die Altbestands-Situation nach der Migration ab. + +```bash +TEST_WEB_PORT=18041 docker compose --env-file /dev/null \ + -p openclockwork-issue41 -f docker-compose.test.yml \ + -f /tmp/openclockwork-41-compose.yml exec -T db \ + psql -U openclockwork -d openclockwork_test -v ON_ERROR_STOP=1 <<'SQL' +INSERT INTO "TimeEntry" ( + "id", "employeeId", "clockIn", "clockOut", "source", "status", + "requiresApproval", "approvalMode", "note", "updatedAt" +) +SELECT fixture.id::uuid, employee.id, fixture.start_at::timestamptz, + fixture.end_at::timestamptz, 'Pwa', 'Pending', true, NULL, + 'Synthetischer Nachtest #41', CURRENT_TIMESTAMP +FROM "Employee" employee +CROSS JOIN (VALUES + ('00000000-0000-4000-8000-000000004101', '2026-10-05T03:00:00Z', '2026-10-05T05:00:00Z'), + ('00000000-0000-4000-8000-000000004102', '2026-10-06T03:00:00Z', '2026-10-06T05:00:00Z'), + ('00000000-0000-4000-8000-000000004103', '2026-10-07T03:00:00Z', '2026-10-07T05:00:00Z') +) AS fixture(id, start_at, end_at) +WHERE employee.email = 'anna.mueller@openclockwork.test' +ON CONFLICT ("id") DO NOTHING; +SQL +``` + +1. Ersten Eintrag durch Marc und Hannah wie oben freigeben. Erwartet: + unveränderte ID/Zeiten, je eine Entscheidung, kein Bypass wegen + `approvalMode=null`. +2. Zweiten Eintrag mit einer echten Notiz ablehnen; beide Ablehnungsstufen + getrennt prüfen (bei Bedarf einen weiteren freien synthetischen Eintrag + anlegen). Ohne nichtleere Begründung ist Ablehnen gesperrt. Nach Ablehnung + ist weder Managerfreigabe noch HR-Bestätigung nachträglich erlaubt. +3. Beim dritten Eintrag als Hannah vorübergehend Annas Managerzuordnung + entfernen oder einen nicht als Manager berechtigten Account zuweisen. + Erwartet: erklärter blockierter Zustand mit Aufforderung, dass HR eine + aktive Führungskraft zuordnet. Keine direkte HR-Bestätigung möglich. + Gültige Zuordnung wiederherstellen: erst Manager, dann HR. +4. Nach Managerfreigabe eine **andere** aktive Führungskraft zuordnen: + alte Führungskraft verliert Zugriff; alter HR-Tab darf nicht bestätigen. + Der neue Manager muss den aktuellen Stand freigeben, anschließend HR. + +### C. Negative Fälle, Änderungen und Konkurrenz + +Netzwerkdetails ausschließlich im isolierten Stack verwenden. Keine Bearer-Token +in Dokumente oder Issues übernehmen. + +`GET /api/timeentries/approvals?scope=own` liefert nur eigene Einträge; +`scope=review` liefert berechtigte Team-/HR-Sicht. Jeder Datensatz enthält +`revision`, `state`, `canDecide` und die Buchung. Entscheidung: + +```http +POST /api/timeentries//approval +Content-Type: application/json + +{ + "revision": 0, + "stage": "Manager", + "outcome": "Approve", + "note": "Synthetisch geprüft" +} +``` + +Für HR die **neu gelesene** Revision und `stage=HR` verwenden. +Ablehnung: `outcome=Reject`, nichtleere Notiz. Verlauf: +`GET /api/timeentries//approval-history`. + +| Versuch | Erwartung | +| --- | --- | +| Ohne Login | 401 | +| Beschäftigte lesen Review-Liste | 403; eigene Status-/Verlaufssicht bleibt möglich | +| Fremde Beschäftigte oder teamfremder Manager entscheiden/lesen Verlauf | 403 | +| `actorId` oder `employeeId` im Entscheidungsbody fälschen | Keine Berechtigungsänderung; Identität ausschließlich aus JWT | +| HR als Manager entscheiden / Manager als HR entscheiden | 403 | +| HR-Stufe vor Managerfreigabe | 409 | +| Eigene Buchung als Manager oder HR selbst freigeben | 403; andere zuständige Personen erforderlich | +| Offene, verworfene, bereits genehmigte/abgelehnte oder nicht genehmigungspflichtige Buchung entscheiden | 409 | +| Fehlende/negative Revision, ungültige Stufe | 400 | +| Veraltete Revision oder wiederholte Entscheidung | 409; keine zweite erfolgreiche Entscheidung, kein doppeltes Auditereignis | +| Zwei Managerentscheidungen oder zwei HR-Entscheidungen gleichzeitig | Genau ein Erfolg, einmal 409 | +| Buchungsänderung gleichzeitig mit Entscheidung auf derselben alten Revision | Genau ein Schreibvorgang erfolgreich; anderer 409 | +| Managerfreigabe, danach Projekt/Tätigkeit/Notiz ändern | Zurück zur Managerprüfung; alter HR-Stand 409, Verlauf enthält Invalidierung | +| Vollständig freigegebene Außenrahmenbuchung nachträglich ändern | Erneut Manager → HR, keine übernommene Endfreigabe | +| Freigegebene Außenrahmenbuchung aufteilen / Projektzeitbereich nachtragen | Betroffene Außenrahmensegmente erneut Managerprüfung; echte Rahmenzeitsegmente behalten normales Verhalten, alte Entscheidungen bleiben im Verlauf auffindbar | +| Abgelehnte Buchung aufteilen | Kein Reinwaschen: beide Segmente bleiben abgelehnt | +| Normale geschlossene Buchung innerhalb des Rahmens | Weiterhin direkt genehmigt; kein neuer Freigabeschritt | +| Separater Solo-Stack: Stempeln, manuell buchen, korrigieren, aufteilen | Unveränderte Solo-Regeln; keine Team-Freigabe alter/aktueller Daten möglich | + +Hannah kann eigene Außenrahmenbuchungen nicht selbst bestätigen; dafür eine +zweite synthetische HR-Person anlegen. Ebenso braucht eine Führungskraft bei +eigenen Buchungen eine andere zugewiesene Führungskraft. Wenn diese Rollen fehlen, +ist zunächst die Organisationszuordnung zu vervollständigen, nicht eine Stufe zu +überspringen. Danach ursprüngliche Plan-/Managerzuordnungen wiederherstellen. + +### Automatische Verifikation #41 + +Ausgeführt am 07.11.2026. Isolierter PostgreSQL-16-Container +`openclockwork-issue-41-db`, Loopback-Port `15441`, ausschließlich synthetische +Datenbanken `openclockwork_test_41` und `openclockwork_upgrade_41`. Keine +bestehende Datenbank zurückgesetzt. Toolchain: pnpm 9.15.9; API-/Web-Gesamttests +zusätzlich unter Node 20.20.2 (CI-Version), übrige lokale Gates unter Node 26.12.1. + +| Gate | Tatsächliches Ergebnis | +| --- | --- | +| `TZ=Europe/Berlin pnpm nx e2e api-e2e --testPathPatterns=time-entry-approvals` | **27/27 neue API-/Service-Regressionen bestanden** | +| `TZ=Europe/Berlin pnpm nx e2e api-e2e` unter Node 20.20.2 | **331/331 Tests, 38 Suites bestanden** | +| `TZ=UTC pnpm nx run-many -t test -p web,shared --run --skip-nx-cache` unter Node 20.20.2 | **Web 234/234 in 35 Dateien**, **Shared 88/88 in 7 Dateien bestanden**, ohne Cache | +| `pnpm nx run-many -t typecheck,lint -p api,web,shared,api-e2e` | **Alle vier Typechecks und Lint erfolgreich**; 0 Fehler, 13 bestehende Warnungen in unveränderten Dateien, keine neue Warnung | +| `pnpm nx run-many -t build -p api,web,shared` getrennt von Typechecks | **Alle drei Builds erfolgreich**, einschließlich Web/PWA; unveränderter Shared-Build aus Nx-Cache | +| `pnpm verify:api` mit isolierter Test-`DATABASE_URL` | **Erfolgreich**; finale OpenAPI-Datei und generierter Client bleiben identisch zur gestagten Fassung | +| Baseline-Migrationen aus akzeptiertem #42 → `node ops/upgrade-smoke.mjs seed` → neue Migration → `node ops/upgrade-smoke.mjs verify` | **Erfolgreich**; echte historische Pending-Buchung bleibt mit Revision 7 und null Signaturmarkern erhalten, keine erfundene Freigabe | +| `git diff --check` | **Erfolgreich** | + +Der Upgrade-Sentinel wurde in einer eigenen frischen Datenbank angelegt: +`PRISMA_BASE_DIR=/Users/patrickschiller/openclockwork-issue-42 pnpm prisma migrate deploy --config ops/prisma-upgrade-baseline.config.ts`, +danach Seed, Migration und Verifikation auf derselben isolierten Upgrade-URL. +Der erweiterte Upgrade-Smoke wird auch von der regulären PR-CI ausgeführt. + +Abgedeckt sind reale Manager→HR-Entscheidungen und Ablehnungen, Pflichtnotiz, +Tokenidentität statt Body-Identität, Team-/Rollen-/Selbstfreigabegrenzen, +fehlende/inaktive/falsch berechtigte Manager, Neuzuordnung, Altbestand, +offene/terminale/in-frame/Solo-Einträge, doppelte/stale/parallele Entscheidungen, +Änderungsrennen, Änderung nach Endfreigabe, Split-/Projektbereichs-Invalidierung +und auffindbarer Verlauf selbst nach Verschieben eines alten Segments in den +Rahmen. Bestehende Solo-/Terminal-/Tagesblock-/Antragsfälle liefen vollständig mit. +Die Oberfläche wird mit React Testing Library geprüft, einschließlich DE/EN, +Mitarbeiterhinweisen, Rollenaktionen, Fehlern und Verlauf. + +Die zusätzlichen historischen Listeneinträge erforderten ID-basierte statt +positionsabhängige Assertions in zwei neuen Tests. Ein Zwischenlauf unter +Node 26 zeigte außerdem einen einzelnen lokalen Socket-`ETIMEDOUT`; der +abschließende vollständige Node-20-Lauf bestand ohne Wiederholungslogik oder +Produktänderung dafür. + +Der manuelle Docker-/Browser-Durchlauf aus dieser Anleitung wurde **nicht** +als durchgeführt oder bestanden gewertet. Keine Release-, Tag-, Versions-, +Merge- oder Deployment-Aktion. + +Zum Stoppen mit Datenerhalt: + +```bash +TEST_WEB_PORT=18041 docker compose --env-file /dev/null \ + -p openclockwork-issue41 -f docker-compose.test.yml \ + -f /tmp/openclockwork-41-compose.yml stop +``` + +Kein `down -v`, kein erneutes Seeden beim Fortsetzen. #43, #45 und #44 sind +noch nicht Bestandteil dieses Pakets. diff --git a/ops/upgrade-smoke.mjs b/ops/upgrade-smoke.mjs index 87abafb..5a99435 100644 --- a/ops/upgrade-smoke.mjs +++ b/ops/upgrade-smoke.mjs @@ -14,6 +14,7 @@ if (!databaseUrl || !['seed', 'verify'].includes(mode)) { const employeeId = '00000000-0000-4000-8000-000000000014'; const timeEntryId = '00000000-0000-4000-8000-000000000114'; +const pendingEntryId = '00000000-0000-4000-8000-000000000115'; const scheduleId = '00000000-0000-4000-8000-000000000214'; const terminalId = '00000000-0000-4000-8000-000000000314'; const fallbackEmployeeId = '00000000-0000-4000-8000-000000000414'; @@ -109,12 +110,46 @@ try { [JSON.stringify(legacyBreakRules), timeEntryId], ); } + // This row exists before the new approval columns and must remain pending, + // with no fabricated manager sign-off after migration. + await client.query( + `INSERT INTO "TimeEntry" ( + "id", "employeeId", "clockIn", "clockOut", "source", "status", + "requiresApproval", "revision", "approvalMode", "updatedAt" + ) VALUES ($1, $2, TIMESTAMP '2026-01-05 06:00:00', + TIMESTAMP '2026-01-05 08:00:00', 'Pwa', 'Pending', true, 7, NULL, CURRENT_TIMESTAMP)`, + [pendingEntryId, fallbackEmployeeId], + ); await client.query('COMMIT'); console.log( 'Inserted upgrade sentinels for employees, policies, time entry, and terminal.', ); } else { + const pending = await client.query( + `SELECT "status"::text, "requiresApproval", "revision", "approvalMode", + "frameApproverId", "frameApprovedRevision", + "clockIn" = TIMESTAMP '2026-01-05 06:00:00' AS "clockInPreserved", + "clockOut" = TIMESTAMP '2026-01-05 08:00:00' AS "clockOutPreserved" + FROM "TimeEntry" WHERE "id" = $1`, + [pendingEntryId], + ); + assert.deepEqual( + pending.rows, + [ + { + status: 'Pending', + requiresApproval: true, + revision: 7, + approvalMode: null, + frameApproverId: null, + frameApprovedRevision: null, + clockInPreserved: true, + clockOutPreserved: true, + }, + ], + 'Legacy pending entries must start at manager review without altering working time.', + ); const sentinel = await client.query( `SELECT e."personalNo", diff --git a/prisma/migrations/20261107031000_time_entry_frame_approval/migration.sql b/prisma/migrations/20261107031000_time_entry_frame_approval/migration.sql new file mode 100644 index 0000000..51a1fe9 --- /dev/null +++ b/prisma/migrations/20261107031000_time_entry_frame_approval/migration.sql @@ -0,0 +1,10 @@ +-- Existing pending Team entries intentionally keep NULL markers: they start +-- at manager review, never at HR or Approved. Historical Solo/in-frame entries +-- and their status, revision, timestamps and audit records remain unchanged. +ALTER TABLE "TimeEntry" + ADD COLUMN "frameApprovedRevision" INTEGER, + ADD COLUMN "frameApproverId" UUID; + +ALTER TABLE "TimeEntry" ADD CONSTRAINT "TimeEntry_frameApproverId_fkey" + FOREIGN KEY ("frameApproverId") REFERENCES "Employee"("id") + ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index dbf4114..f797c96 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -152,6 +152,7 @@ model Employee { soloPolicies SoloPolicy[] personalDays PersonalDay[] timeEntryAudits TimeEntryAudit[] + frameApprovedEntries TimeEntry[] @relation("FrameApprover") installationEvents InstallationEvent[] createdAt DateTime @default(now()) updatedAt DateTime @updatedAt @@ -204,6 +205,10 @@ model TimeEntry { source EntrySource @default(Manual) status EntryStatus @default(Open) requiresApproval Boolean @default(false) + /// Manager sign-off is valid only for the exact revision and current manager. + frameApprovedRevision Int? + frameApproverId String? @db.Uuid + frameApprover Employee? @relation("FrameApprover", fields: [frameApproverId], references: [id], onDelete: SetNull) billable Boolean @default(false) revision Int @default(0) voidedAt DateTime?