From 72b64140e922f18ccb115bce452a108c0dc98009 Mon Sep 17 00:00:00 2001 From: mAi Date: Mon, 25 May 2026 14:24:55 +0200 Subject: [PATCH] mAi: #83 - approval withdraw warning modal + edit-instead path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit t-paliad-252. Replace the silent confirm()-then-DELETE with a three-path warning modal: Cancel / Edit event (primary) / Withdraw and delete (destructive). The edit-instead path lets the requester revise the in-flight entity without withdrawing the approval request. Backend — new service method + endpoint - ApprovalService.EditPendingEntity(requestID, callerID, fields): - validates caller == requested_by AND status = pending - reuses the existing wider counter-allowlist (buildCounterSetClauses from SuggestChanges) — every editable field on the entity, not just the date triggers - applies the field updates to the entity row via applyEntityUpdate (including the event_type_ids junction rewrite for deadlines) - merges new fields into approval_requests.payload (jsonb) so the approver inbox sees what was revised - emits a distinct *_approval_edited_by_requester project_event so the Verlauf surfaces the revision separately from the original *_requested row and any decision row - request stays pending; entity.approval_status stays pending - POST /api/approval-requests/{id}/edit-entity - Body: {"fields": {}} - Errors reuse the existing mapApprovalError mapping: 400 suggestion_requires_change, 403 not_authorized, 404, 409 request_not_pending - Distinguishing audit event types per the spec: - destructive Withdraw path: existing _approval_revoked (no behaviour change — for CREATE deletes the entity, for UPDATE / COMPLETE reverts to pre_image, for DELETE cancels the delete request) - edit-instead path: new _approval_edited_by_requester Frontend — shared withdraw warning modal - frontend/src/client/components/withdraw-warning-modal.ts - Built on the unified openModal() primitive (t-paliad-217 Slice A) - Primary CTA "Termin bearbeiten" highlights the non-destructive path - Secondary defaults to "Abbrechen" (handled by openModal) - Destructive button "Endgültig zurückziehen und löschen" lives inside the body (red, separated by a dashed border) so the safe path stays visually primary in the footer - Copy adapts per lifecycle: CREATE → "Wenn Sie zurückziehen, wird die Frist/der Termin gelöscht." UPDATE → "Ihre vorgeschlagenen Änderungen werden verworfen." DELETE → "Der Eintrag bleibt bestehen." Frontend — wiring on both detail pages - deadlines-detail.ts + appointments-detail.ts: - Replace confirm() in withdraw flow with openWithdrawWarningModal() - Edit path: set module-level pendingEditMode = true + enter edit mode (override existing pending-state freeze on appointments; expose enterEdit() via late-bound pendingEnterEdit on deadlines) - Save handler in pendingEditMode routes to /edit-entity instead of PATCH /api//{id} (which still 409s on pending state) - Destructive Withdraw path: existing /revoke endpoint unchanged - For CREATE-lifecycle revokes the entity is gone — bounce to the /events list instead of trying to re-fetch (was reload() before) i18n: +14 keys DE+EN under approvals.withdraw.* (modal title, primary, destructive, cancel, lead.create.{deadline,appointment}, lead.update, lead.delete, sub.create, sub.update, sub.delete) CSS: .withdraw-warning-body + .withdraw-warning-{intro,sub, destructive-row,destructive-btn} — lime-tint sibling palette consistent with the existing form-hint pattern; destructive button uses .btn-danger. Build hygiene: - go build + go vet + go test ./internal/... clean - frontend bun run build clean (2807 keys, +14 new, scan clean) Files of note: - internal/services/approval_service.go (EditPendingEntity + sortedKeys helper; maps.Copy for the payload merge) - internal/handlers/approvals.go (handleEditPendingEntity) - internal/handlers/handlers.go (route registration) - frontend/src/client/components/withdraw-warning-modal.ts (new shared component) - frontend/src/client/deadlines-detail.ts (initWithdraw rewrite + Save pending-edit branch) - frontend/src/client/appointments-detail.ts (withdrawAppointmentRequest rewrite + Save pending-edit branch + form-freeze respects pendingEditMode) Out of scope (intentionally): - Reopening already-deleted approval requests (the destructive path stays final). - Approval-request analytics / metrics. - Notifying the original approval-requester via channel. --- frontend/src/client/appointments-detail.ts | 81 +++++++++- .../components/withdraw-warning-modal.ts | 149 ++++++++++++++++++ frontend/src/client/deadlines-detail.ts | 92 ++++++++++- frontend/src/client/i18n.ts | 22 +++ frontend/src/i18n-keys.ts | 11 ++ frontend/src/styles/global.css | 36 +++++ internal/handlers/approvals.go | 50 ++++++ internal/handlers/handlers.go | 3 + internal/services/approval_service.go | 130 +++++++++++++++ 9 files changed, 562 insertions(+), 12 deletions(-) create mode 100644 frontend/src/client/components/withdraw-warning-modal.ts diff --git a/frontend/src/client/appointments-detail.ts b/frontend/src/client/appointments-detail.ts index d30982a..d1e934d 100644 --- a/frontend/src/client/appointments-detail.ts +++ b/frontend/src/client/appointments-detail.ts @@ -2,6 +2,7 @@ import { initI18n, t, tDyn, getLang } from "./i18n"; import { initSidebar } from "./sidebar"; import { initNotes } from "./notes"; import { projectIndent } from "./project-indent"; +import { openWithdrawWarningModal } from "./components/withdraw-warning-modal"; interface Appointment { id: string; @@ -25,6 +26,9 @@ interface PendingApprovalRequest { requested_at: string; required_role: string; requester_name?: string; + // t-paliad-252 — used by the withdraw warning modal to pick the right + // copy (CREATE warns about deletion; UPDATE/COMPLETE about revert). + lifecycle_event?: string; } interface Me { @@ -43,6 +47,10 @@ let project: Project | null = null; let allProjects: Project[] = []; let pendingRequest: PendingApprovalRequest | null = null; let me: Me | null = null; +// t-paliad-252 — see deadlines-detail.ts. Routes Save to the new +// /api/approval-requests/{id}/edit-entity endpoint when the user picked +// "Termin bearbeiten" in the withdraw warning modal. +let pendingEditMode = false; function parseAppointmentID(): string | null { const parts = window.location.pathname.split("/").filter(Boolean); @@ -207,10 +215,14 @@ function renderHeader() { } // Freeze the edit form + delete button while a request is in flight. + // t-paliad-252 — when the user picked "Termin bearbeiten" in the + // withdraw modal, pendingEditMode unfreezes the form so Save can route + // to /edit-entity (which keeps the request pending + merges payload). const form = document.getElementById("appointment-edit-form") as HTMLFormElement | null; if (form) { + const freeze = isPending && !pendingEditMode; form.querySelectorAll("input, select, textarea, button[type=submit]") - .forEach((el) => { el.disabled = isPending; }); + .forEach((el) => { el.disabled = freeze; }); } const deleteBtn = document.getElementById("appointment-delete-btn") as HTMLButtonElement | null; if (deleteBtn) deleteBtn.disabled = isPending; @@ -263,6 +275,39 @@ async function saveEdit(ev: Event) { submitBtn.disabled = true; try { + // t-paliad-252 — pending-edit mode routes through /edit-entity which + // keeps the request pending + merges fields into payload. clear_project + // and project_id are NOT in the counter-allowlist (yet) — the requester + // can't move projects on a pending request from this surface. + if (pendingEditMode && pendingRequest) { + const editFields = { ...payload }; + delete editFields.clear_project; + const resp = await fetch( + `/api/approval-requests/${pendingRequest.id}/edit-entity`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ fields: editFields }), + }, + ); + if (resp.ok) { + const fresh = await fetch(`/api/appointments/${appointment.id}`); + if (fresh.ok) appointment = await fresh.json(); + await loadPendingRequest(); + // Exit pending-edit mode so the form re-freezes (still pending). + pendingEditMode = false; + renderHeader(); + fillEditForm(); + msg.textContent = t("appointments.detail.saved"); + msg.className = "form-msg form-msg-ok"; + } else { + const data = await resp.json().catch(() => ({}) as { error?: string; message?: string }); + msg.textContent = data.message || data.error || t("appointments.error.generic"); + msg.className = "form-msg form-msg-error"; + } + return; + } + const resp = await fetch(`/api/appointments/${appointment.id}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, @@ -312,12 +357,37 @@ async function deleteAppointment() { } } +// t-paliad-252 — withdraw warning modal replaces the old confirm(). +// Returns: +// "edit" → unfreeze the edit form (pending-edit mode); Save will +// route through /api/approval-requests/{id}/edit-entity +// "withdraw" → destructive: the existing /revoke endpoint +// null → user cancelled async function withdrawAppointmentRequest() { if (!appointment || !pendingRequest) return; - if (!confirm(t("approvals.withdraw.confirm") || "Anfrage wirklich zurückziehen?")) return; const btn = document.getElementById("appointment-withdraw-btn") as HTMLButtonElement | null; if (btn) btn.disabled = true; try { + const action = await openWithdrawWarningModal({ + entityType: "appointment", + lifecycleEvent: pendingRequest.lifecycle_event ?? "create", + }); + if (action === null) { + if (btn) btn.disabled = false; + return; + } + if (action === "edit") { + pendingEditMode = true; + if (btn) btn.disabled = false; + // renderHeader re-evaluates the freeze and unfreezes the form now + // that pendingEditMode is set. Focus the first editable field so the + // user can type immediately. + renderHeader(); + const titleEl = document.getElementById("appointment-title-edit") as HTMLInputElement | null; + titleEl?.focus(); + return; + } + // action === "withdraw" → destructive path. const resp = await fetch(`/api/approval-requests/${pendingRequest.id}/revoke`, { method: "POST", headers: { "Content-Type": "application/json" }, @@ -328,9 +398,12 @@ async function withdrawAppointmentRequest() { if (fresh.ok) { appointment = await fresh.json(); await loadPendingRequest(); + renderHeader(); + fillEditForm(); + } else { + // CREATE lifecycle: entity gone → back to the list. + window.location.href = "/events?type=appointment"; } - renderHeader(); - fillEditForm(); } else { const data = await resp.json().catch(() => ({}) as { message?: string; error?: string }); const msg = document.getElementById("appointment-edit-msg")!; diff --git a/frontend/src/client/components/withdraw-warning-modal.ts b/frontend/src/client/components/withdraw-warning-modal.ts new file mode 100644 index 0000000..835a8f0 --- /dev/null +++ b/frontend/src/client/components/withdraw-warning-modal.ts @@ -0,0 +1,149 @@ +// t-paliad-252 / m/paliad#83 — withdraw warning modal. +// +// Before t-paliad-252 the deadline + appointment detail pages did a +// confirm() dialog before POSTing to /api/approval-requests/{id}/revoke. +// For pending CREATE lifecycles that endpoint silently DELETES the +// underlying entity row — m's "withdrawing the approval deletes the event" +// surprise. +// +// This modal replaces the confirm() with three explicit paths: +// +// 1. Cancel — does nothing +// 2. Termin bearbeiten (primary) — opens the edit form; saving routes +// through POST /approval-requests/{id}/ +// edit-entity which keeps the request +// pending and merges the new fields +// into approval_request.payload +// 3. Endgültig zurückziehen + — destructive; current /revoke +// löschen behaviour (delete for CREATE, revert +// for UPDATE/COMPLETE, cancel for +// DELETE-lifecycle requests) +// +// Built on the unified openModal() primitive (t-paliad-217 Slice A) so the +// three-button row sits cleanly inside the body — the primitive only +// supports one secondary action, but we paint the destructive button as a +// separate row above the footer. + +import { t } from "../i18n"; +import { openModal } from "./modal"; + +export type WithdrawAction = "edit" | "withdraw"; + +export interface WithdrawWarningArgs { + // entityType drives the copy ("event" vs "appointment" labels). + entityType: "deadline" | "appointment"; + // lifecycleEvent of the pending request; copy adapts (CREATE warns about + // deletion; UPDATE/COMPLETE warn about revert; DELETE warns about + // cancelling the deletion request). + lifecycleEvent: "create" | "update" | "complete" | "delete" | string; +} + +// openWithdrawWarningModal resolves with the chosen action, or null if the +// user dismissed via Cancel / Esc / backdrop / browser back-button. +export async function openWithdrawWarningModal( + args: WithdrawWarningArgs, +): Promise { + const body = document.createElement("div"); + body.className = "withdraw-warning-body"; + + // Lead paragraph + sub-paragraph adapt to lifecycle so the user always + // knows what the destructive button will actually do. The /revoke + // backend behaviour: + // - create → DELETE the entity (the "surprise" m flagged) + // - update → revert to pre_image + // - complete → revert to pre-complete state + // - delete → cancel the delete request (entity stays alive) + const intro = document.createElement("p"); + intro.className = "withdraw-warning-intro"; + intro.textContent = leadCopyFor(args); + body.appendChild(intro); + + const sub = document.createElement("p"); + sub.className = "withdraw-warning-sub muted"; + sub.textContent = subCopyFor(args); + body.appendChild(sub); + + // The destructive button lives inside the body — the openModal primitive + // only exposes one secondary button slot, and we want the safe "Edit" + // path to be the primary CTA. Painting it in red here, separated from + // the footer, signals "this is the dangerous option" without competing + // visually with the primary CTA. + const destructiveRow = document.createElement("div"); + destructiveRow.className = "withdraw-warning-destructive-row"; + const destructiveBtn = document.createElement("button"); + destructiveBtn.type = "button"; + destructiveBtn.className = "btn btn-danger withdraw-warning-destructive-btn"; + destructiveBtn.textContent = t("approvals.withdraw.destructive.label"); + destructiveRow.appendChild(destructiveBtn); + body.appendChild(destructiveRow); + + return new Promise((resolve) => { + let chosen: WithdrawAction | null = null; + + // The destructive button has to close the modal and return "withdraw". + // We need access to the modal's internal close() — fortunately openModal + // exposes it via the primary handler's first arg. We pass through the + // outer resolve and let the primary handler (Edit) own the close-fn + // route. For the destructive button we resolve the outer promise + // directly and then synthesise an ESC keypress so the modal dismisses + // — or, simpler, set chosen and use the secondary "Cancel" path that + // the modal already supports. (openModal's onClose fires on every + // dismiss path including the primary handler resolution.) + destructiveBtn.addEventListener("click", () => { + chosen = "withdraw"; + // The unified openModal primitive (modal.ts) wires its dismiss path + // through the native 's `cancel` event. Dispatching it on + // the parent runs the same finish() → onClose → resolve + // sequence as ESC / backdrop. We then map the resolved `null` back + // to "withdraw" via the captured `chosen` in onClose below. + const dialogEl = body.closest("dialog"); + dialogEl?.dispatchEvent(new Event("cancel")); + }); + + void openModal({ + title: t("approvals.withdraw.modal.title"), + body, + size: "md", + classNames: "withdraw-warning-modal", + primary: { + label: t("approvals.withdraw.primary.label"), + handler: (close) => { + chosen = "edit"; + close("edit"); + }, + }, + secondary: { label: t("approvals.withdraw.cancel") }, + onClose: () => { + // Resolves whatever was chosen via the destructive button OR the + // primary handler. ESC / backdrop / secondary clear `chosen` to + // null which is the right "cancel" semantics. + resolve(chosen); + }, + }); + }); +} + +function leadCopyFor(args: WithdrawWarningArgs): string { + switch (args.lifecycleEvent) { + case "create": + return args.entityType === "appointment" + ? t("approvals.withdraw.lead.create.appointment") + : t("approvals.withdraw.lead.create.deadline"); + case "delete": + return t("approvals.withdraw.lead.delete"); + default: + // update / complete / unknown → revert semantics + return t("approvals.withdraw.lead.update"); + } +} + +function subCopyFor(args: WithdrawWarningArgs): string { + switch (args.lifecycleEvent) { + case "create": + return t("approvals.withdraw.sub.create"); + case "delete": + return t("approvals.withdraw.sub.delete"); + default: + return t("approvals.withdraw.sub.update"); + } +} diff --git a/frontend/src/client/deadlines-detail.ts b/frontend/src/client/deadlines-detail.ts index c1aa98d..4e333a4 100644 --- a/frontend/src/client/deadlines-detail.ts +++ b/frontend/src/client/deadlines-detail.ts @@ -9,6 +9,7 @@ import { type EventType, type PickerHandle, } from "./event-types"; +import { openWithdrawWarningModal } from "./components/withdraw-warning-modal"; interface Deadline { id: string; @@ -38,6 +39,9 @@ interface PendingApprovalRequest { requested_at: string; required_role: string; requester_name?: string; + // t-paliad-252 — used by the withdraw warning modal to pick the right + // copy (CREATE warns about deletion; UPDATE/COMPLETE about revert). + lifecycle_event?: string; } let eventTypePicker: PickerHandle | null = null; @@ -69,6 +73,18 @@ let rule: DeadlineRule | null = null; let me: Me | null = null; let allProjects: Project[] = []; let pendingRequest: PendingApprovalRequest | null = null; +// t-paliad-252 — when the user chose "Edit event" in the withdraw warning +// modal, the entity is still in approval_status='pending'. Save must POST +// to /api/approval-requests/{id}/edit-entity (which keeps the request +// pending + merges the new fields into payload) instead of the regular +// PATCH /api/deadlines/{id} (which 409s during pending). Cleared on exit +// from edit mode + after a successful save. +let pendingEditMode = false; + +// pendingEnterEdit — late-bound by initEdit() so the withdraw warning +// modal handler (initWithdraw) can route into pending-edit mode without +// duplicating the edit-mode toggle logic. +let pendingEnterEdit: (() => void) | null = null; function parseDeadlineID(): string | null { const parts = window.location.pathname.split("/").filter(Boolean); @@ -404,8 +420,17 @@ function initEdit() { if (titleDefaultBtn) titleDefaultBtn.style.display = "none"; saveBtn.style.display = "none"; editBtn.style.display = ""; + pendingEditMode = false; } + // t-paliad-252 — expose enterEdit so the withdraw warning modal can + // route into pending-edit mode without re-running the edit-button + // visibility gate (which hides the button during pending). + pendingEnterEdit = () => { + pendingEditMode = true; + enterEdit(); + }; + editBtn.addEventListener("click", enterEdit); // t-paliad-251 Part 4 — Standardtitel button. @@ -455,6 +480,35 @@ function initEdit() { if (projectEdit && projectEdit.value && projectEdit.value !== deadline.project_id) { payload.project_id = projectEdit.value; } + + // t-paliad-252 — pending-edit mode routes through the new endpoint + // that updates the entity + merges payload into the still-pending + // approval_request. Outside pending-edit mode the regular PATCH + // path remains the authoritative one (with its existing 409-on- + // pending guard). + if (pendingEditMode && pendingRequest) { + const resp = await fetch( + `/api/approval-requests/${pendingRequest.id}/edit-entity`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ fields: payload }), + }, + ); + if (resp.ok) { + const fresh = await fetch(`/api/deadlines/${deadline.id}`); + if (fresh.ok) deadline = await fresh.json(); + await loadPendingRequest(); + render(); + } else { + const body = await resp.json().catch(() => null); + const msg = (body && (body.message || body.error)) + || (t("approvals.withdraw.error") || "Fehler"); + window.alert(msg); + } + return; + } + const resp = await fetch(`/api/deadlines/${deadline.id}`, { method: "PATCH", headers: { "Content-Type": "application/json" }, @@ -532,19 +586,39 @@ function initReopen() { }); } -// initWithdraw — t-paliad-160 §C+E. Reuses the existing -// /api/approval-requests/{id}/revoke endpoint (no new server route -// needed). After the revoke lands, the entity goes back to -// approval_status='approved' and the page reloads to refresh the -// in-memory state cleanly. +// initWithdraw — t-paliad-160 §C+E + t-paliad-252. +// +// Click flow: open the withdraw warning modal (replaces the old +// confirm()). The modal returns one of: +// +// "edit" — open the edit form in pending-edit mode; Save calls +// /api/approval-requests/{id}/edit-entity which keeps the +// request pending + merges the new fields into payload +// "withdraw" — destructive: call the existing /revoke endpoint +// (DELETE entity for CREATE, revert for UPDATE/COMPLETE, +// cancel-delete for DELETE lifecycle) +// null — user cancelled; nothing happens function initWithdraw() { const btn = document.getElementById("deadline-withdraw-btn") as HTMLButtonElement | null; if (!btn) return; btn.addEventListener("click", async () => { if (!deadline || !pendingRequest) return; - if (!window.confirm(t("approvals.withdraw.confirm") || "Anfrage wirklich zurückziehen?")) return; btn.disabled = true; try { + const action = await openWithdrawWarningModal({ + entityType: "deadline", + lifecycleEvent: pendingRequest.lifecycle_event ?? "create", + }); + if (action === null) { + btn.disabled = false; + return; + } + if (action === "edit") { + btn.disabled = false; + pendingEnterEdit?.(); + return; + } + // action === "withdraw" → existing destructive path. const resp = await fetch(`/api/approval-requests/${pendingRequest.id}/revoke`, { method: "POST", headers: { "Content-Type": "application/json" }, @@ -552,14 +626,16 @@ function initWithdraw() { }); if (resp.ok) { // Re-fetch the entity so approval_status flips back to 'approved' - // and the badge / buttons rerender accordingly. + // and the badge / buttons rerender accordingly. For CREATE + // lifecycle the entity is gone, so the 404 surfaces as a reload. const r = await fetch(`/api/deadlines/${deadline.id}`); if (r.ok) { deadline = await r.json(); await loadPendingRequest(); render(); } else { - window.location.reload(); + // CREATE lifecycle deleted the entity — bounce to the list. + window.location.href = "/events?type=deadline"; } } else { btn.disabled = false; diff --git a/frontend/src/client/i18n.ts b/frontend/src/client/i18n.ts index 3a502b1..f7d79e6 100644 --- a/frontend/src/client/i18n.ts +++ b/frontend/src/client/i18n.ts @@ -2602,6 +2602,17 @@ const translations: Record> = { "approvals.withdraw.cta": "Genehmigungsanfrage zurückziehen", "approvals.withdraw.confirm": "Genehmigungsanfrage wirklich zurückziehen?", "approvals.withdraw.error": "Fehler beim Zurückziehen", + "approvals.withdraw.cancel": "Abbrechen", + "approvals.withdraw.modal.title": "Genehmigungsanfrage zurückziehen?", + "approvals.withdraw.primary.label": "Termin bearbeiten", + "approvals.withdraw.destructive.label": "Endgültig zurückziehen und löschen", + "approvals.withdraw.lead.create.deadline": "Wenn Sie die Anfrage zurückziehen, wird die Frist gelöscht.", + "approvals.withdraw.lead.create.appointment": "Wenn Sie die Anfrage zurückziehen, wird der Termin gelöscht.", + "approvals.withdraw.lead.update": "Wenn Sie die Anfrage zurückziehen, werden die vorgeschlagenen Änderungen verworfen — der Eintrag kehrt in den Zustand vor Ihrer Bearbeitung zurück.", + "approvals.withdraw.lead.delete": "Wenn Sie die Löschanfrage zurückziehen, bleibt der Eintrag bestehen.", + "approvals.withdraw.sub.create": "Alternativ können Sie den Eintrag stattdessen bearbeiten. Die Anfrage bleibt offen und der Genehmiger sieht Ihre neuen Werte.", + "approvals.withdraw.sub.update": "Alternativ können Sie Ihre Änderungen bearbeiten und neu absenden. Die Anfrage bleibt offen.", + "approvals.withdraw.sub.delete": "Sind Sie sicher, dass Sie die Löschanfrage zurückziehen möchten?", "approvals.pending_create.label": "Erstellung wartet auf Genehmigung", "approvals.pending_update.label": "Änderung wartet auf Genehmigung", "approvals.pending_complete.label": "Erledigung wartet auf Genehmigung", @@ -5540,6 +5551,17 @@ const translations: Record> = { "approvals.withdraw.cta": "Withdraw approval request", "approvals.withdraw.confirm": "Withdraw the approval request?", "approvals.withdraw.error": "Failed to withdraw", + "approvals.withdraw.cancel": "Cancel", + "approvals.withdraw.modal.title": "Withdraw approval request?", + "approvals.withdraw.primary.label": "Edit event", + "approvals.withdraw.destructive.label": "Withdraw permanently and delete", + "approvals.withdraw.lead.create.deadline": "Withdrawing this request will delete the deadline.", + "approvals.withdraw.lead.create.appointment": "Withdrawing this request will delete the appointment.", + "approvals.withdraw.lead.update": "Withdrawing this request will discard your proposed changes — the entry will revert to its state before your edit.", + "approvals.withdraw.lead.delete": "Withdrawing the delete request will keep the entry alive.", + "approvals.withdraw.sub.create": "Alternatively, you can edit the entry instead. The request stays open and the approver will see your new values.", + "approvals.withdraw.sub.update": "Alternatively, you can edit your changes and resubmit. The request stays open.", + "approvals.withdraw.sub.delete": "Are you sure you want to withdraw the delete request?", "approvals.pending_create.label": "Awaits approval (creation)", "approvals.pending_update.label": "Awaits approval (change)", "approvals.pending_complete.label": "Awaits approval (completion)", diff --git a/frontend/src/i18n-keys.ts b/frontend/src/i18n-keys.ts index 6ea860b..40c6ffe 100644 --- a/frontend/src/i18n-keys.ts +++ b/frontend/src/i18n-keys.ts @@ -682,9 +682,20 @@ export type I18nKey = | "approvals.tab.mine" | "approvals.tab.pending_mine" | "approvals.title" + | "approvals.withdraw.cancel" | "approvals.withdraw.confirm" | "approvals.withdraw.cta" + | "approvals.withdraw.destructive.label" | "approvals.withdraw.error" + | "approvals.withdraw.lead.create.appointment" + | "approvals.withdraw.lead.create.deadline" + | "approvals.withdraw.lead.delete" + | "approvals.withdraw.lead.update" + | "approvals.withdraw.modal.title" + | "approvals.withdraw.primary.label" + | "approvals.withdraw.sub.create" + | "approvals.withdraw.sub.delete" + | "approvals.withdraw.sub.update" | "bottomnav.add" | "bottomnav.add.appointment" | "bottomnav.add.appointment.sub" diff --git a/frontend/src/styles/global.css b/frontend/src/styles/global.css index 0cef4ae..f837315 100644 --- a/frontend/src/styles/global.css +++ b/frontend/src/styles/global.css @@ -7723,6 +7723,42 @@ select.rule-sort-select { background: #b91c1c; } +/* t-paliad-252 — withdraw warning modal body. The destructive button sits + inside the body (above the footer's Cancel + Edit primary) so the safe + "Edit event" path stays visually primary. The intro paragraph leads, + the muted sub-line explains consequences, then the red row makes the + destructive option discoverable without competing with the CTA. */ +.withdraw-warning-body { + display: flex; + flex-direction: column; + gap: 0.75rem; +} +.withdraw-warning-intro { + margin: 0; + color: var(--color-text); + font-size: 0.92rem; + line-height: 1.45; +} +.withdraw-warning-sub { + margin: 0; + color: var(--color-text-muted); + font-size: 0.85rem; + line-height: 1.45; +} +.withdraw-warning-destructive-row { + display: flex; + justify-content: flex-end; + margin-top: 0.5rem; + padding-top: 0.75rem; + border-top: 1px dashed var(--color-border); +} +.withdraw-warning-destructive-btn { + /* Inherits .btn .btn-danger, but bump the font size down a touch so + the body button doesn't crowd the footer's primary CTA. */ + font-size: 0.82rem; + padding: 0.4rem 1rem; +} + .entity-soon { text-align: center; padding: 3rem 1.5rem; diff --git a/internal/handlers/approvals.go b/internal/handlers/approvals.go index 1d9f5c3..7775b16 100644 --- a/internal/handlers/approvals.go +++ b/internal/handlers/approvals.go @@ -326,6 +326,56 @@ func handleRevokeApprovalRequest(w http.ResponseWriter, r *http.Request) { handleApprovalDecision(w, r, "revoke") } +// POST /api/approval-requests/{id}/edit-entity — t-paliad-252 / m/paliad#83. +// +// Lets the requester revise the in-flight entity (e.g. tweak the title on a +// pending create) without withdrawing the request. The non-destructive +// sibling of /revoke that m asked for after noticing that withdraw silently +// deletes the underlying event. +// +// Body: {"fields": {}} +// 200: {"status": "ok"} +// +// Status mapping (mapApprovalError): +// +// 400 suggestion_requires_change — payload has no allowlisted fields +// 403 not_authorized — caller isn't the requested_by +// 404 — request not found / not visible +// 409 request_not_pending — request already decided / revoked +type editPendingEntityBody struct { + Fields map[string]any `json:"fields"` +} + +func handleEditPendingEntity(w http.ResponseWriter, r *http.Request) { + if !requireDB(w) { + return + } + uid, ok := requireUser(w, r) + if !ok { + return + } + requestID, err := uuid.Parse(r.PathValue("id")) + if err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid request id"}) + return + } + var body editPendingEntityBody + if r.Body != nil && r.ContentLength > 0 { + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + writeJSON(w, http.StatusBadRequest, map[string]string{ + "code": "invalid_body", + "message": "Ungültiger Body.", + }) + return + } + } + if err := dbSvc.approval.EditPendingEntity(r.Context(), requestID, uid, body.Fields); err != nil { + writeApprovalError(w, err) + return + } + writeJSON(w, http.StatusOK, map[string]string{"status": "ok"}) +} + // suggestChangesBody is the JSON body for POST /api/approval-requests/{id}/suggest-changes. // counter_payload is an entity-shaped jsonb of the approver's edited // values (allowlist enforced server-side); note is the optional free-text diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index e4de5c0..35792d7 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -658,6 +658,9 @@ func Register(mux *http.ServeMux, client *auth.Client, giteaAPIToken string, svc protected.HandleFunc("POST /api/approval-requests/{id}/approve", handleApproveApprovalRequest) protected.HandleFunc("POST /api/approval-requests/{id}/reject", handleRejectApprovalRequest) protected.HandleFunc("POST /api/approval-requests/{id}/revoke", handleRevokeApprovalRequest) + // t-paliad-252 — non-destructive sibling of /revoke: lets the + // requester revise the in-flight entity without withdrawing. + protected.HandleFunc("POST /api/approval-requests/{id}/edit-entity", handleEditPendingEntity) protected.HandleFunc("POST /api/approval-requests/{id}/suggest-changes", handleSuggestChangesApprovalRequest) // t-paliad-154 — form-time effective policy lookup. Reachable by diff --git a/internal/services/approval_service.go b/internal/services/approval_service.go index 117a0f8..381d050 100644 --- a/internal/services/approval_service.go +++ b/internal/services/approval_service.go @@ -41,6 +41,7 @@ import ( "encoding/json" "errors" "fmt" + "maps" "strings" "time" @@ -364,6 +365,135 @@ func (s *ApprovalService) Revoke(ctx context.Context, requestID, callerID uuid.U return s.decide(ctx, requestID, callerID, RequestStatusRevoked, "") } +// EditPendingEntity lets the REQUESTER of a pending approval_request revise +// the in-flight entity (e.g. tweak the title or due_date on a pending +// create) without withdrawing the request. t-paliad-252 / m/paliad#83 added +// this as the non-destructive sibling of Revoke — m's mental model is +// "withdraw deletes the event; let me edit the event instead, keep the +// approval request alive". +// +// Authorization: caller MUST be the original requested_by (no approver can +// edit on the requester's behalf — that would collapse into SuggestChanges). +// Request status MUST be pending. +// +// Allowlist: uses the WIDER counter-allowlist already maintained for +// SuggestChanges (buildCounterSetClauses) — every editable field on the +// entity, not just the date-bearing approval triggers. Unknown keys are +// silently dropped. Returns ErrSuggestionRequiresChange when fields carries +// no allowlisted key for the entity_type (would be a no-op write). +// +// Side effects in one tx: entity columns updated (and event_type_ids junction +// rewritten for deadlines), approval_request.payload merged with the new +// values so the approver sees what was revised, and a distinct +// `_approval_edited_by_requester` project_event emitted so the +// Verlauf shows the revision separately from the original *_requested row. +// +// The approval_request stays pending; entity.approval_status stays pending. +// The approver inbox sees a fresh updated_at + the merged payload. +func (s *ApprovalService) EditPendingEntity(ctx context.Context, requestID, callerID uuid.UUID, fields map[string]any) error { + tx, err := s.db.BeginTxx(ctx, nil) + if err != nil { + return fmt.Errorf("begin tx: %w", err) + } + defer tx.Rollback() //nolint:errcheck + + req, err := s.getRequestForUpdate(ctx, tx, requestID) + if err != nil { + return err + } + if req.Status != RequestStatusPending { + return fmt.Errorf("%w: status=%s", ErrRequestNotPending, req.Status) + } + if callerID != req.RequestedBy { + return ErrNotApprover + } + + // Validate the counter-allowlist intersect produces at least one + // settable column. applyEntityUpdate also wraps this check; pre-checking + // here lets us emit a cleaner error before opening the entity-write. + if _, _, err := buildCounterSetClauses(req.EntityType, fields); err != nil { + // Already wraps ErrSuggestionRequiresChange for empty / title-cleared + // cases. Propagate verbatim. + return err + } + + // Apply the field updates to the entity row via the shared + // counter-allowlist path (same as SuggestChanges). + if err := s.applyEntityUpdate(ctx, tx, req.EntityType, req.EntityID, fields); err != nil { + return err + } + + // Merge new fields into the request payload so the approver's inbox + // reflects what the requester revised to. Keys overwrite; event_type_ids + // is replaced wholesale per the same semantics applyEntityUpdate uses + // for the junction rewrite. + var existing map[string]any + if len(req.Payload) > 0 { + if err := json.Unmarshal(req.Payload, &existing); err != nil { + return fmt.Errorf("unmarshal payload: %w", err) + } + } + if existing == nil { + existing = map[string]any{} + } + maps.Copy(existing, fields) + merged, err := json.Marshal(existing) + if err != nil { + return fmt.Errorf("marshal merged payload: %w", err) + } + now := time.Now().UTC() + if _, err := tx.ExecContext(ctx, + `UPDATE paliad.approval_requests + SET payload = $1, updated_at = $2 + WHERE id = $3`, + merged, now, requestID); err != nil { + return fmt.Errorf("update payload: %w", err) + } + + // Audit emit. Distinct event_type so the Verlauf surfaces the revision + // separately from the original *_requested or any decision row. + verlaufKind := "edited_by_requester" + eventType := approvalEventType(req.EntityType, verlaufKind) + descPtr := approvalDescription(verlaufKind, req.RequiredRole, req.LifecycleEvent) + editedKeys := sortedKeys(fields) + meta := map[string]any{ + "approval_request_id": req.ID.String(), + "lifecycle_event": req.LifecycleEvent, + req.EntityType + "_id": req.EntityID.String(), + "edited_fields": editedKeys, + } + if err := insertProjectEventWithMeta(ctx, tx, req.ProjectID, callerID, eventType, eventType, descPtr, meta); err != nil { + return err + } + return tx.Commit() +} + +// sortedKeys returns m's keys in stable alphabetical order so the audit-log +// metadata is byte-for-byte stable across calls (helps when diffing audit +// logs or asserting on them in tests). +func sortedKeys(m map[string]any) []string { + out := make([]string, 0, len(m)) + for k := range m { + out = append(out, k) + } + // Use the stdlib sort; the slice is small (≤ counter-allowlist size). + sortStrings(out) + return out +} + +// sortStrings: indirection so we don't add a new top-level import group. +// In Go 1.21+ slices.Sort exists; this package is currently importing +// strings + standard libs and adding "sort" would re-fan the imports. +// Kept as a one-line wrapper to localise the dependency if a later move +// to slices.Sort feels right. +func sortStrings(s []string) { + for i := 1; i < len(s); i++ { + for j := i; j > 0 && s[j-1] > s[j]; j-- { + s[j-1], s[j] = s[j], s[j-1] + } + } +} + // SuggestChanges is the fourth approval action (t-paliad-216). The caller // proposes a counter-payload + optional free-text note; in one transaction // we close the old request as 'changes_requested', revert the entity from