Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions .changeset/approval-band-honors-lock-record.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
---
"@object-ui/react": minor
"@object-ui/plugin-detail": minor
"@object-ui/app-shell": minor
"@object-ui/i18n": minor
---

fix(detail): the approval band honors the node's `lockRecord` instead of assuming every approval locks (#2902)

A record detail page treated "a pending approval request exists" as "this
record is locked". An approval node declares `lockRecord` (default `true`), and
on `lockRecord: false` the server keeps accepting writes for the whole time
that node waits — so the console was asserting a lock the backend did not
enforce.

The label was the smaller half of it. The same conflated signal fed `canEdit`,
so the record-level inline-edit session was suppressed too: no pencils,
`enter()` a no-op. On a single-approver step — a department head or plant
manager, exactly the case `lockRecord: false` exists for, where the approver is
meant to fill in the missing detail before deciding — the capability was
unreachable from the UI. And a flow chaining nodes with different policies drew
one identical band for "edit freely" and "the server will reject your save with
`RECORD_LOCKED`", so the two states were indistinguishable until Save failed.

Approval state is now two signals:

- **`approvalPending`** — an approval is running. Drives the band and the recall
button, both meaningful whether or not the record is editable.
- **`locked`** — that approval also forbids edits, from the pending node's
`lock_record` (framework#3814, read off the same `node_config_json` snapshot
the server's record-lock hook reads).

The band renders two states: amber lock + "Locked for approval", or sky clock +
"In approval · editable", each with its own tooltip. Recall moved out of the
locked branch — an editable pending approval is just as recallable. Inline
editing stays live in the editable state.

`InlineEditProvider` takes a new optional `approvalPending` prop, defaulting to
`locked`, so a host that threads only `locked` renders exactly as before. The
record's `approval_status` field remains the fallback for backends with no
approvals API; it carries no node granularity, so it still reads as locked — as
does a pending request from a backend too old to report the policy.

New `detail.approvalPendingEditable` / `detail.approvalPendingTooltip` keys are
translated in all ten locales.
61 changes: 61 additions & 0 deletions packages/app-shell/src/hooks/useRecordApprovals.locking.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import { describe, it, expect } from 'vitest';
import { recordLockedByApproval, type ApprovalRequestLite } from './useRecordApprovals';

/**
* Record-lock policy of a pending approval (objectui#2902).
*
* "A pending approval request exists" and "the record is locked" are different
* facts: an approval node declares `lockRecord`, and the server's record-lock
* hook lets writes through for the whole time a `lockRecord: false` node waits.
* The console used to equate them, which disabled inline editing and showed a
* "Locked for approval" badge on records the server would have let the user
* edit — exactly the single-approver steps the flag exists for.
*/

const req = (over: Partial<ApprovalRequestLite> = {}): ApprovalRequestLite => ({
id: 'r1',
process_name: 'flow:budget_approval',
object_name: 'showcase_project',
record_id: 'p1',
status: 'pending',
...over,
});

describe('recordLockedByApproval (objectui#2902)', () => {
it('is not locked with no pending request', () => {
expect(recordLockedByApproval(null)).toBe(false);
expect(recordLockedByApproval(undefined)).toBe(false);
});

it('is locked when the pending node declares lockRecord', () => {
expect(recordLockedByApproval(req({ lock_record: true }))).toBe(true);
});

it('is NOT locked when the pending node opted out — the #2902 regression', () => {
expect(recordLockedByApproval(req({ lock_record: false }))).toBe(false);
});

it('fails closed when the backend does not report the policy', () => {
// Pre-framework#3814: the flag was never sent. Assuming "unlocked" there
// would offer an edit the server rejects with RECORD_LOCKED, so the
// unknown must read as locked.
expect(recordLockedByApproval(req())).toBe(true);
});

it('reads each node independently as a flow advances', () => {
// The issue's repro: node A unlocked, node B locked. One request per node,
// each carrying its own policy — the band must change between them.
const nodeA = req({ id: 'rA', current_step: 'manager_review', lock_record: false });
const nodeB = req({ id: 'rB', current_step: 'exec_review', lock_record: true });
expect(recordLockedByApproval(nodeA)).toBe(false);
expect(recordLockedByApproval(nodeB)).toBe(true);
});
});
30 changes: 30 additions & 0 deletions packages/app-shell/src/hooks/useRecordApprovals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,36 @@ export interface ApprovalRequestLite {
pending_approvers?: string[] | null;
submitted_at?: string;
completed_at?: string | null;
/**
* Whether THIS pending node locks the record from edits (objectui#2902).
* The approval node's `lockRecord` policy, surfaced by the server from the
* same `node_config_json` snapshot its record-lock `beforeUpdate` hook reads
* — so the badge we render and the rule the server applies agree.
*
* `undefined` on a pre-framework#3814 backend, which never sent the flag.
* Callers must fail CLOSED there (treat as locked): offering an edit the
* server then rejects with `RECORD_LOCKED` is worse than hiding one it would
* have allowed. See {@link recordLockedByApproval}.
*/
lock_record?: boolean;
}

/**
* Does an open approval request lock its record from edits?
*
* A pending request is NOT the same thing as a locked record: an approval node
* may declare `lockRecord: false`, and the server then lets the record be
* edited while that node waits (a single-approver step where the approver is
* meant to fill in the missing detail is the motivating case). Treating "has a
* pending request" as "locked" mislabels those nodes and hides an edit the
* server would have accepted — objectui#2902.
*
* Fails closed on both unknowns: no request at all is not a lock, but a request
* from a backend too old to report the policy is.
*/
export function recordLockedByApproval(request: ApprovalRequestLite | null | undefined): boolean {
if (!request) return false;
return request.lock_record !== false;
}

interface UseRecordApprovalsResult {
Expand Down
46 changes: 34 additions & 12 deletions packages/app-shell/src/views/RecordDetailView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import { resolveActionParams } from '../utils/resolveActionParams';
import { useRecordBreadcrumbTitle } from '../context/NavigationContext';
import type { FeedItem } from '@object-ui/types';
import type { ActionDef, ActionParamDef } from '@object-ui/core';
import { useRecordApprovals } from '../hooks/useRecordApprovals';
import { useRecordApprovals, recordLockedByApproval } from '../hooks/useRecordApprovals';
import { RecordAttachmentsPanel } from './RecordAttachmentsPanel';
import { RecordPermissionAssignmentsRenderer } from './metadata-admin/RecordPermissionAssignmentsRenderer';
import { getRecordDisplayName } from '../utils';
Expand Down Expand Up @@ -868,14 +868,32 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
void approvalsRef.current.refresh();
}, [recordInvalidationNonce]);

// The shared lock signal for the inline-edit session: the record's own
// `approval_status` (what the DetailView lock band keys off) OR an open
// pending request from the approvals API — the latter catches backends
// that lock via the request record without materializing the field.
const approvalLocked =
// Approval state is TWO signals, not one (objectui#2902).
//
// `approvalPending` — an approval is in flight on this record. Drives the
// status band and the recall affordance, which are meaningful whether or not
// the record is editable.
//
// `approvalLocked` — that approval also forbids edits. The approval node's
// `lockRecord` policy decides this, and the server enforces exactly that
// policy in its record-lock `beforeUpdate` hook. Conflating the two (which
// this used to do) made every `lockRecord: false` node claim the record was
// locked while the server happily accepted writes — and, worse than the
// wrong label, `canEdit` below then suppressed the inline-edit affordances
// entirely, so the "approver may fill in the missing detail" case the flag
// exists for was unreachable from the console.
//
// The record's own `approval_status` field stays a fallback for backends
// that mirror status onto the record but expose no approvals API. It carries
// no node granularity, so it can only mean "locked" — the conservative read,
// and the same one `recordLockedByApproval` applies to a pre-framework#3814 backend.
const approvalStatusPending =
(pageRecord as any)?.approval_status === 'pending' ||
(pageRecord as any)?.approval_status === 'in_approval' ||
!!approvals.pendingRequest;
(pageRecord as any)?.approval_status === 'in_approval';
const approvalPending = approvalStatusPending || !!approvals.pendingRequest;
const approvalLocked = approvals.pendingRequest
? recordLockedByApproval(approvals.pendingRequest)
: approvalStatusPending;

const approvalHandler = useCallback(async (action: ActionDef) => {
const target = action.target || action.name;
Expand Down Expand Up @@ -1916,13 +1934,17 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri
hides the pencil affordances and no-ops `enter()`, so users can't
type into a draft that Save would reject with RECORD_LOCKED.
`locked` surfaces the approval lock as its own signal so the
DetailView "Locked for approval" band renders from the SAME
dual-source `approvalLocked` that gated `canEdit` — engaging even
on backends that track the lock via approval requests only and
never materialize an `approval_status` field (objectui#2618). */}
DetailView approval band renders from the SAME dual-source
`approvalLocked` that gated `canEdit` — engaging even on backends
that track the lock via approval requests only and never
materialize an `approval_status` field (objectui#2618).
`approvalPending` rides alongside it so a node that declares
`lockRecord: false` still shows its band and recall button while
leaving the record editable (objectui#2902). */}
<InlineEditProvider
canEdit={resolveRecordHeaderActionGates(objectDef, effectiveApiOperations).edit && !approvalLocked}
locked={approvalLocked}
approvalPending={approvalPending}
lockedReason={t('detail.lockedTooltip', {
defaultValue: 'This record has a pending approval request; editing is locked',
})}
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/ar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ const ar = {
saving: "جارٍ الحفظ…",
lockedByApproval: "مقفل للموافقة",
lockedTooltip: "يحتوي هذا السجل على طلب موافقة معلق؛ التعديل مقفل",
approvalPendingEditable: "قيد الموافقة · قابل للتعديل",
approvalPendingTooltip: "يحتوي هذا السجل على طلب موافقة معلق، لكن هذه الخطوة لا تزال تسمح بالتعديل",
cancelApproval: "إلغاء الموافقة",
cancelApprovalInFlight: "جارٍ الإلغاء…",
cancelApprovalTooltip: "إلغاء طلب الموافقة المعلق لفتح قفل السجل",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/de.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ const de = {
saving: "Speichern…",
lockedByApproval: "Zur Genehmigung gesperrt",
lockedTooltip: "Dieser Datensatz hat eine ausstehende Genehmigungsanfrage; die Bearbeitung ist gesperrt",
approvalPendingEditable: "In Genehmigung · bearbeitbar",
approvalPendingTooltip: "Dieser Datensatz hat eine ausstehende Genehmigungsanfrage; dieser Schritt erlaubt weiterhin die Bearbeitung",
cancelApproval: "Genehmigung zurückziehen",
cancelApprovalInFlight: "Zurückziehen…",
cancelApprovalTooltip: "Ausstehende Genehmigungsanfrage zurückziehen, um den Datensatz zu entsperren",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -680,6 +680,8 @@ const en = {
editInlineHint: 'Double-click to edit',
lockedByApproval: 'Locked for approval',
lockedTooltip: 'This record has a pending approval request; editing is locked',
approvalPendingEditable: 'In approval · editable',
approvalPendingTooltip: 'This record has a pending approval request; this step still allows editing',
cancelApproval: 'Recall approval',
cancelApprovalInFlight: 'Recalling…',
cancelApprovalTooltip: 'Recall the pending approval request to unlock this record',
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ const es = {
saving: "Guardando…",
lockedByApproval: "Bloqueado para aprobación",
lockedTooltip: "Este registro tiene una solicitud de aprobación pendiente; la edición está bloqueada",
approvalPendingEditable: "En aprobación · editable",
approvalPendingTooltip: "Este registro tiene una solicitud de aprobación pendiente; este paso todavía permite la edición",
cancelApproval: "Cancelar aprobación",
cancelApprovalInFlight: "Cancelando…",
cancelApprovalTooltip: "Cancelar la solicitud de aprobación pendiente para desbloquear el registro",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/fr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ const fr = {
saving: "Enregistrement…",
lockedByApproval: "Verrouillé pour approbation",
lockedTooltip: "Cet enregistrement a une demande d'approbation en attente ; la modification est verrouillée",
approvalPendingEditable: "En approbation · modifiable",
approvalPendingTooltip: "Cet enregistrement a une demande d'approbation en attente ; cette étape autorise encore la modification",
cancelApproval: "Annuler l'approbation",
cancelApprovalInFlight: "Annulation…",
cancelApprovalTooltip: "Annuler la demande d'approbation en attente pour déverrouiller l'enregistrement",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/ja.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,8 @@ const ja = {
saving: "保存中…",
lockedByApproval: "承認のためロック中",
lockedTooltip: "このレコードには承認待ちのリクエストがあります。編集はロックされています",
approvalPendingEditable: "承認中 · 編集可能",
approvalPendingTooltip: "このレコードには承認待ちのリクエストがありますが、このステップでは編集できます",
cancelApproval: "承認を取り消す",
cancelApprovalInFlight: "取り消し中…",
cancelApprovalTooltip: "承認待ちリクエストを取り消してレコードのロックを解除する",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/ko.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ const ko = {
saving: "저장 중…",
lockedByApproval: "승인을 위해 잠김",
lockedTooltip: "이 레코드에 대기 중인 승인 요청이 있습니다. 편집이 잠겨 있습니다",
approvalPendingEditable: "승인 진행 중 · 편집 가능",
approvalPendingTooltip: "이 레코드에 대기 중인 승인 요청이 있지만 이 단계에서는 편집할 수 있습니다",
cancelApproval: "승인 취소",
cancelApprovalInFlight: "취소 중…",
cancelApprovalTooltip: "대기 중인 승인 요청을 취소하여 레코드 잠금 해제",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/pt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,8 @@ const pt = {
saving: "Salvando…",
lockedByApproval: "Bloqueado para aprovação",
lockedTooltip: "Este registro tem uma solicitação de aprovação pendente; a edição está bloqueada",
approvalPendingEditable: "Em aprovação · editável",
approvalPendingTooltip: "Este registro tem uma solicitação de aprovação pendente; esta etapa ainda permite a edição",
cancelApproval: "Cancelar aprovação",
cancelApprovalInFlight: "Cancelando…",
cancelApprovalTooltip: "Cancelar a solicitação de aprovação pendente para desbloquear o registro",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/ru.ts
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,8 @@ const ru = {
saving: "Сохранение…",
lockedByApproval: "Заблокировано для согласования",
lockedTooltip: "У этой записи есть ожидающий запрос на согласование; редактирование заблокировано",
approvalPendingEditable: "На согласовании · редактирование доступно",
approvalPendingTooltip: "У этой записи есть ожидающий запрос на согласование, но этот шаг всё ещё разрешает редактирование",
cancelApproval: "Отменить согласование",
cancelApprovalInFlight: "Отмена…",
cancelApprovalTooltip: "Отмените ожидающий запрос на согласование, чтобы разблокировать запись",
Expand Down
2 changes: 2 additions & 0 deletions packages/i18n/src/locales/zh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,8 @@ const zh = {
editInlineHint: '双击编辑',
lockedByApproval: '审批中已锁定',
lockedTooltip: '该记录有待审批的请求,编辑已被锁定',
approvalPendingEditable: '审批中 · 可编辑',
approvalPendingTooltip: '该记录有待审批的请求,但当前审批节点仍允许编辑',
cancelApproval: '撤回审批',
cancelApprovalInFlight: '撤回中…',
cancelApprovalTooltip: '撤回当前的待审批请求以解除记录锁定',
Expand Down
Loading
Loading