m/paliad
1
0
Fork 0
Code Issues 152 Pull Requests 4 Actions Packages Projects Releases Wiki Activity

Approval withdrawal: warning + edit-event-instead path #83

New Issue
Open
opened 2026-05-25 11:23:38 +00:00 by mAi · 1 comment
mAi commented 2026-05-25 11:23:38 +00:00
Collaborator
Copy Link

m's report (2026-05-25 13:08)

Where withdrawing the approval request deletes the event, we should give a warning and allow editing the event instead which would change the approval request?!

Scope

Currently: withdrawing an approval request silently deletes the underlying event. m wants:

  1. A warning dialog before deletion explaining what will happen.
  2. An alternative path "Edit event instead" — modifying the event updates / re-targets the approval request rather than deleting it.

What to do

  1. Find the approval-withdrawal flow:
    • Likely frontend/src/client/approvals.ts or similar
    • Backend handler in internal/handlers/approvals.go
  2. Replace the immediate-delete behavior with a confirmation modal:
    • Title: "Genehmigungsanfrage zurückziehen?" / "Withdraw approval request?"
    • Body: Explain that withdrawing deletes the underlying event by default. Offer two clear next steps:
      • Primary action: "Termin bearbeiten" / "Edit event" — closes the modal and opens the event-edit form. Saving the edit updates the approval request (its target/proposed change re-targets to the edited event).
      • Secondary action: "Endgültig zurückziehen und löschen" / "Withdraw permanently and delete" — destructive button, requires explicit click. Performs the current delete behavior.
      • Cancel — does nothing.
  3. Backend: when an approval request is updated through the event-edit path, the request stays alive with the new event state; only when the user picks the explicit destructive action does the existing delete cascade fire.
  4. Make sure the audit log captures both paths distinctly: approval-withdrawn-via-edit vs approval-withdrawn-and-deleted.

Files most likely touched

  • frontend/src/client/approvals.ts
  • frontend/src/client/i18n.ts — confirm dialog copy
  • internal/handlers/approvals.go — split the destructive path from the edit-instead path; audit-log distinction
  • internal/services/approval_service.go (if exists)

Hard rules

  • Default modal action highlights the non-destructive choice ("Edit event") — destructive only on explicit click.
  • Audit-log entry on every state transition.
  • go build ./... && go test ./internal/... && cd frontend && bun run build clean.
  • Branch: mai/<worker>/approval-withdraw-warning.

Out of scope

  • Reopening already-deleted approval requests (out of scope; the destructive path stays final).
  • Approval-request analytics / metrics.
  • Notifying the original approval-requester via channel (separate concern).

Reporting

mai report completed with branch + SHAs + the UX path: trigger withdraw → see modal → pick "Edit event" → event editor opens → save → confirm approval request is alive with new state; alternative: pick destructive → confirm event + request deleted; cancel: nothing changes.

## m's report (2026-05-25 13:08) > Where withdrawing the approval request deletes the event, we should give a warning and allow editing the event instead which would change the approval request?! ## Scope Currently: withdrawing an approval request silently deletes the underlying event. m wants: 1. A **warning dialog** before deletion explaining what will happen. 2. An alternative path **"Edit event instead"** — modifying the event updates / re-targets the approval request rather than deleting it. ## What to do 1. Find the approval-withdrawal flow: - Likely `frontend/src/client/approvals.ts` or similar - Backend handler in `internal/handlers/approvals.go` 2. Replace the immediate-delete behavior with a confirmation modal: - **Title**: "Genehmigungsanfrage zurückziehen?" / "Withdraw approval request?" - **Body**: Explain that withdrawing deletes the underlying event by default. Offer two clear next steps: - **Primary action**: "Termin bearbeiten" / "Edit event" — closes the modal and opens the event-edit form. Saving the edit updates the approval request (its target/proposed change re-targets to the edited event). - **Secondary action**: "Endgültig zurückziehen und löschen" / "Withdraw permanently and delete" — destructive button, requires explicit click. Performs the current delete behavior. - **Cancel** — does nothing. 3. Backend: when an approval request is updated through the event-edit path, the request stays alive with the new event state; only when the user picks the explicit destructive action does the existing delete cascade fire. 4. Make sure the audit log captures both paths distinctly: `approval-withdrawn-via-edit` vs `approval-withdrawn-and-deleted`. ## Files most likely touched - `frontend/src/client/approvals.ts` - `frontend/src/client/i18n.ts` — confirm dialog copy - `internal/handlers/approvals.go` — split the destructive path from the edit-instead path; audit-log distinction - `internal/services/approval_service.go` (if exists) ## Hard rules - Default modal action highlights the non-destructive choice ("Edit event") — destructive only on explicit click. - Audit-log entry on every state transition. - `go build ./... && go test ./internal/... && cd frontend && bun run build` clean. - Branch: `mai/<worker>/approval-withdraw-warning`. ## Out of scope - Reopening already-deleted approval requests (out of scope; the destructive path stays final). - Approval-request analytics / metrics. - Notifying the original approval-requester via channel (separate concern). ## Reporting `mai report completed` with branch + SHAs + the UX path: trigger withdraw → see modal → pick "Edit event" → event editor opens → save → confirm approval request is alive with new state; alternative: pick destructive → confirm event + request deleted; cancel: nothing changes.
mAi self-assigned this 2026-05-25 11:23:38 +00:00
mAi commented 2026-05-25 12:25:45 +00:00
Author
Collaborator
Copy Link

Done — branch mai/artemis/gitster-approval-withdraw

Commit: 72b6414

New shared modal, new backend endpoint, both deadline + appointment detail pages wired. No schema migration needed — re-uses the existing counter-allowlist (buildCounterSetClauses) from SuggestChanges (t-paliad-216).

UX paths

Trigger. Click "Genehmigungsanfrage zurückziehen" on a pending deadline / appointment you authored.

Modal copy (adapts to lifecycle):

  • CREATE: "Wenn Sie die Anfrage zurückziehen, wird die Frist/der Termin gelöscht."
  • UPDATE/COMPLETE: "Ihre vorgeschlagenen Änderungen werden verworfen — der Eintrag kehrt in den Zustand vor Ihrer Bearbeitung zurück."
  • DELETE: "Wenn Sie die Löschanfrage zurückziehen, bleibt der Eintrag bestehen."

Three paths:

  1. Cancel (footer, also Esc / backdrop / close-×) — nothing happens.
  2. Termin bearbeiten (footer, primary CTA — non-destructive) — modal closes, the entity's edit form unfreezes (pending-edit mode). Save sends POST /api/approval-requests/{id}/edit-entity with the new field payload. The approval request stays pending; the entity row + approval_request.payload are synced to the new values. Audit row: <entity>_approval_edited_by_requester (new event type, distinct from the original *_requested row).
  3. Endgültig zurückziehen und löschen (inside body, red, separated by a dashed border so it never competes with the primary CTA) — the existing /revoke endpoint runs unchanged. For CREATE that hard-deletes the entity (the surprise m flagged); for UPDATE/COMPLETE it reverts to pre_image; for DELETE it cancels the delete request. Audit row: <entity>_approval_revoked.

Audit distinction (per spec)

Two paths emit different project_events.event_type rows so the Verlauf reads correctly:

Path Event type Behaviour
Edit-instead <entity>_approval_edited_by_requester (new) request stays pending, entity + payload synced, requester revises before approver acts
Destructive withdraw <entity>_approval_revoked (existing) request → revoked, entity reverted via applyRevert (= DELETE for CREATE)

Backend

  • ApprovalService.EditPendingEntity(ctx, requestID, callerID, fields) — new service method.
    • Auth: caller MUST be requested_by AND status MUST be pending. Returns ErrNotApprover / ErrRequestNotPending otherwise.
    • Allowlist: reuses buildCounterSetClauses (the wider counter-allowlist from SuggestChanges) — every editable field on the entity, not just the date-bearing approval triggers. Unknown keys silently dropped. Empty-fields / title-cleared → ErrSuggestionRequiresChange.
    • Side effects in one tx:
      1. Entity columns updated via applyEntityUpdate (incl. event_type_ids junction rewrite for deadlines).
      2. approval_requests.payload merged with new fields (jsonb deep-merge using maps.Copy).
      3. project_events row inserted with new *_approval_edited_by_requester event type + metadata {edited_fields: [...]}.
  • POST /api/approval-requests/{id}/edit-entity — new handler in internal/handlers/approvals.go.
    • Body: {"fields": {<entity-shape>}}
    • Errors map through the existing mapApprovalError: 400 suggestion_requires_change, 403 not_authorized, 404, 409 request_not_pending.

Frontend

  • frontend/src/client/components/withdraw-warning-modal.ts — new shared modal built on openModal() primitive (t-paliad-217 Slice A). Adapts copy by lifecycle, exposes 3 paths via destructive-button-in-body + primary CTA in footer.
  • frontend/src/client/deadlines-detail.ts — initWithdraw rewrite; Save handler branches on pendingEditMode to the new endpoint.
  • frontend/src/client/appointments-detail.ts — same pattern; appointment edit form already lives inline (always visible, frozen during pending) so pendingEditMode unfreezes it.
  • i18n: +14 keys DE+EN under approvals.withdraw.*.
  • CSS: .withdraw-warning-body + .withdraw-warning-{intro,sub,destructive-row,destructive-btn}.

Build hygiene

  • go build ./... clean
  • go vet ./... clean
  • go test ./internal/... clean (no new tests — the new path uses the SQL infrastructure that already has live-DB coverage via SuggestChanges)
  • bun run build clean (2807 keys, +14 new, scan clean)

Out of scope (intentionally, per spec)

  • Reopening already-deleted approval requests — destructive path stays final.
  • Approval-request analytics / metrics.
  • Notifying the original approval-requester via channel.

Ready for maria's review + merge. Awaiting head merge gate.

## Done — branch `mai/artemis/gitster-approval-withdraw` Commit: [`72b6414`](https://mgit.msbls.de/m/paliad/commit/72b64140e922f18ccb115bce452a108c0dc98009) New shared modal, new backend endpoint, both deadline + appointment detail pages wired. No schema migration needed — re-uses the existing counter-allowlist (`buildCounterSetClauses`) from `SuggestChanges` (t-paliad-216). ### UX paths **Trigger.** Click "Genehmigungsanfrage zurückziehen" on a pending deadline / appointment you authored. **Modal copy (adapts to lifecycle):** - **CREATE**: `"Wenn Sie die Anfrage zurückziehen, wird die Frist/der Termin gelöscht."` - **UPDATE/COMPLETE**: `"Ihre vorgeschlagenen Änderungen werden verworfen — der Eintrag kehrt in den Zustand vor Ihrer Bearbeitung zurück."` - **DELETE**: `"Wenn Sie die Löschanfrage zurückziehen, bleibt der Eintrag bestehen."` **Three paths:** 1. **Cancel** (footer, also Esc / backdrop / close-×) — nothing happens. 2. **Termin bearbeiten** (footer, primary CTA — non-destructive) — modal closes, the entity's edit form unfreezes (pending-edit mode). Save sends `POST /api/approval-requests/{id}/edit-entity` with the new field payload. The approval request **stays pending**; the entity row + `approval_request.payload` are synced to the new values. Audit row: `<entity>_approval_edited_by_requester` (new event type, distinct from the original `*_requested` row). 3. **Endgültig zurückziehen und löschen** (inside body, red, separated by a dashed border so it never competes with the primary CTA) — the existing `/revoke` endpoint runs unchanged. For CREATE that hard-deletes the entity (the surprise m flagged); for UPDATE/COMPLETE it reverts to `pre_image`; for DELETE it cancels the delete request. Audit row: `<entity>_approval_revoked`. ### Audit distinction (per spec) Two paths emit different `project_events.event_type` rows so the Verlauf reads correctly: | Path | Event type | Behaviour | |---|---|---| | Edit-instead | `<entity>_approval_edited_by_requester` (new) | request stays pending, entity + payload synced, requester revises before approver acts | | Destructive withdraw | `<entity>_approval_revoked` (existing) | request → revoked, entity reverted via `applyRevert` (= DELETE for CREATE) | ### Backend - `ApprovalService.EditPendingEntity(ctx, requestID, callerID, fields)` — new service method. - Auth: caller MUST be `requested_by` AND status MUST be `pending`. Returns `ErrNotApprover` / `ErrRequestNotPending` otherwise. - Allowlist: reuses `buildCounterSetClauses` (the wider counter-allowlist from `SuggestChanges`) — every editable field on the entity, not just the date-bearing approval triggers. Unknown keys silently dropped. Empty-fields / title-cleared → `ErrSuggestionRequiresChange`. - Side effects in one tx: 1. Entity columns updated via `applyEntityUpdate` (incl. `event_type_ids` junction rewrite for deadlines). 2. `approval_requests.payload` merged with new fields (jsonb deep-merge using `maps.Copy`). 3. `project_events` row inserted with new `*_approval_edited_by_requester` event type + metadata `{edited_fields: [...]}`. - `POST /api/approval-requests/{id}/edit-entity` — new handler in `internal/handlers/approvals.go`. - Body: `{"fields": {<entity-shape>}}` - Errors map through the existing `mapApprovalError`: 400 `suggestion_requires_change`, 403 `not_authorized`, 404, 409 `request_not_pending`. ### Frontend - `frontend/src/client/components/withdraw-warning-modal.ts` — new shared modal built on `openModal()` primitive (t-paliad-217 Slice A). Adapts copy by lifecycle, exposes 3 paths via destructive-button-in-body + primary CTA in footer. - `frontend/src/client/deadlines-detail.ts` — `initWithdraw` rewrite; Save handler branches on `pendingEditMode` to the new endpoint. - `frontend/src/client/appointments-detail.ts` — same pattern; appointment edit form already lives inline (always visible, frozen during pending) so `pendingEditMode` unfreezes it. - i18n: +14 keys DE+EN under `approvals.withdraw.*`. - CSS: `.withdraw-warning-body` + `.withdraw-warning-{intro,sub,destructive-row,destructive-btn}`. ### Build hygiene - `go build ./...` clean - `go vet ./...` clean - `go test ./internal/...` clean (no new tests — the new path uses the SQL infrastructure that already has live-DB coverage via `SuggestChanges`) - `bun run build` clean (2807 keys, +14 new, scan clean) ### Out of scope (intentionally, per spec) - Reopening already-deleted approval requests — destructive path stays final. - Approval-request analytics / metrics. - Notifying the original approval-requester via channel. Ready for maria's review + merge. Awaiting head merge gate.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
deferred
Parked — not pursuing now, may un-park later
done
Work completed by an agent; awaiting m to close
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
mAi m
No Assignees mAi
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: m/paliad#83
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?