From 02cc19726fc28c6115eec958bce7509833a395cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8C=85=E5=91=A8=E6=B6=9B?= Date: Tue, 28 Jul 2026 04:53:55 -0700 Subject: [PATCH 1/2] fix(detail): distinguish "in approval (editable)" from locked, and stop losing write warnings (framework#3794) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The record page told the user the opposite of what the backend does, in both directions. The approval band rendered "Locked for approval" on any pending request, ignoring the node's `lockRecord`. A `lockRecord: false` node is pending WITHOUT locking — that is how an approver amends a record while deciding on it — so the band hid the feature; meanwhile the header Edit button stayed enabled on a genuinely locked record, letting the user fill a form the save would reject with RECORD_LOCKED. `useRecordApprovals` now reads `locks_record` off the pending request (framework#3794), `RecordDetailView` derives `approvalPending` and `approvalLocked` separately and threads both through `InlineEditProvider`, and the band picks the amber lock or a neutral "In approval (editable)" from the host's signals. With no approvals-aware host the `approval_status` fallback is unchanged and still assumes a lock. The write-warning toast (framework #3431/#3455) never fired on `batchTransaction`, which is the record form's save path for a master-detail record — precisely where a user edits a `readonlyWhen`-locked field. It now emits per event, resolving each back to its operation via the response `index`. The toast is localized (en + zh) and worded by reason: a `readonly_when` strip says the field is not editable in this record's current state, not that it is read-only. Finally, the user-state adapter no longer hand-stamps the server-managed `updated_at`, which the server strips and reports — that fired a "Some fields were not saved" warning about a field no user touched, on ordinary page loads. Co-Authored-By: Claude Opus 5 --- .../approval-band-truth-and-write-warnings.md | 51 +++++++++++ .../app-shell/src/hooks/useRecordApprovals.ts | 10 +++ .../src/providers/AdapterProvider.tsx | 65 +++++++++++--- .../app-shell/src/views/RecordDetailView.tsx | 41 +++++++-- packages/data-objectstack/src/index.ts | 36 ++++++++ .../src/onWriteWarning.test.ts | 50 +++++++++++ .../data-objectstack/src/userState.test.ts | 29 ++++++- packages/data-objectstack/src/userState.ts | 16 ++-- packages/i18n/src/locales/en.ts | 6 ++ packages/i18n/src/locales/zh.ts | 6 ++ packages/plugin-detail/src/DetailView.tsx | 85 +++++++++++++------ .../DetailView.approvalBand.test.tsx | 54 +++++++++++- .../plugin-detail/src/useDetailTranslation.ts | 5 ++ .../react/src/context/InlineEditContext.tsx | 26 +++++- 14 files changed, 422 insertions(+), 58 deletions(-) create mode 100644 .changeset/approval-band-truth-and-write-warnings.md diff --git a/.changeset/approval-band-truth-and-write-warnings.md b/.changeset/approval-band-truth-and-write-warnings.md new file mode 100644 index 0000000000..4c619c516c --- /dev/null +++ b/.changeset/approval-band-truth-and-write-warnings.md @@ -0,0 +1,51 @@ +--- +"@object-ui/plugin-detail": minor +"@object-ui/app-shell": minor +"@object-ui/data-objectstack": minor +"@object-ui/react": minor +"@object-ui/i18n": minor +--- + +fix(detail): say what the record actually allows — "in approval (editable)" vs locked, and warn on silently stripped fields (framework#3794) + +The Console reported record writability wrong in both directions during an +approval, so a user had nothing to go on: what they *could* edit said "locked", +and what they *couldn't* said "updated successfully". + +**"Locked for approval" was painted over every pending approval.** The band and +the recall affordance rendered on any open request, ignoring the node's +`lockRecord`. A node declaring `lockRecord: false` is pending *without* locking — +the server's hook lets the write through on purpose, so the approver can amend +the record as part of deciding on it — and the band told them not to bother. The +mirror image was just as bad: on a genuinely locked record the header **Edit** +button stayed live, so the user filled a whole form before the save came back +`RECORD_LOCKED`. + +The pending request is now authoritative for both signals. `useRecordApprovals` +reads `locks_record` (framework#3794, resolved from the same node-config snapshot +the lock hook reads); `RecordDetailView` derives `approvalPending` and +`approvalLocked` separately and threads both through `InlineEditProvider` +(new `approvalPending` prop). The band renders the amber lock only when the +record is really locked, a neutral "In approval (editable)" / 审批中（可编辑） +otherwise, and the header Edit CTA is disabled on a locked record. With no +approvals-aware host, the old `approval_status`-field fallback is unchanged and +still assumes a lock — the safe direction. + +**Silently stripped fields now surface on the record form's save path.** The +adapter emitted a write-warning for `create`/`update` responses carrying +`droppedFields`, but not for `batchTransaction` — which is how the record form +saves a master-detail record, i.e. the one surface where a user actually edits a +`readonlyWhen`-locked field. `batchTransaction` now emits one warning per event, +resolving each back to its operation via the response's `index`. + +The toast itself was hardcoded English and called every strip "read-only". It is +now localized (`detail.writeStripped*`, en + zh) and worded by reason: +`readonly_when` says the field is not editable *in this record's current state*, +which is what actually happened — the field is editable in other states and the +form rendered it as an ordinary input. + +**And it stopped crying wolf.** `createObjectStackUserStateAdapter` hand-stamped +the server-managed `updated_at` on every recents/favorites write, which the +server strips and reports — so the console popped "Some fields were not saved" +about a field no user ever touched, on page loads, drowning the signal the toast +exists for. It no longer sends the column; the server stamps it anyway. diff --git a/packages/app-shell/src/hooks/useRecordApprovals.ts b/packages/app-shell/src/hooks/useRecordApprovals.ts index 6a9611636f..363210ca99 100644 --- a/packages/app-shell/src/hooks/useRecordApprovals.ts +++ b/packages/app-shell/src/hooks/useRecordApprovals.ts @@ -32,6 +32,16 @@ export interface ApprovalRequestLite { pending_approvers?: string[] | null; submitted_at?: string; completed_at?: string | null; + /** + * Whether THIS request's node locks the record for writes (framework #3794). + * Read from the node-config snapshot's `lockRecord` by the same server that + * enforces the lock, so it cannot disagree with the `beforeUpdate` hook. + * + * `false` ⇒ pending but writable — the node deliberately lets the approver + * amend the record while deciding. Absent ⇒ an older server with no opinion; + * callers assume locked (the safe direction). + */ + locks_record?: boolean; } interface UseRecordApprovalsResult { diff --git a/packages/app-shell/src/providers/AdapterProvider.tsx b/packages/app-shell/src/providers/AdapterProvider.tsx index 601c905d9e..bd60df2e5d 100644 --- a/packages/app-shell/src/providers/AdapterProvider.tsx +++ b/packages/app-shell/src/providers/AdapterProvider.tsx @@ -7,29 +7,59 @@ * @module */ -import { useState, useEffect, type ReactNode } from 'react'; +import { useState, useEffect, useRef, type ReactNode } from 'react'; import { toast } from 'sonner'; import { ObjectStackAdapter, type WriteWarningEvent } from '@object-ui/data-objectstack'; import { createAuthenticatedFetch } from '@object-ui/auth'; import { AdapterCtx } from '@object-ui/react'; +import { useObjectTranslation } from '@object-ui/i18n'; import { installSettleSignalGlobal, withSettleSignal } from '../observability/settleSignal'; export { useAdapter } from '@object-ui/react'; +type TranslateFn = (key: string, options?: Record) => string; + /** * Turn a write-warning (framework #3431/#3455) into a human toast. The write - * SUCCEEDED — some caller-supplied fields were legally stripped (read-only / - * locked by state), so we tell the user rather than let it pass silently. + * SUCCEEDED — some caller-supplied fields were legally stripped, so we tell the + * user rather than let it pass silently. + * + * The REASON decides the wording (#3794). `readonly_when` is not "this field is + * read-only" — the field is editable in other states and the form rendered it + * as an ordinary input; what happened is that THIS record's current state locks + * it. Saying "read-only" there sends the user looking for a permission problem + * that doesn't exist. Fields are named by their machine name: the event carries + * no labels, and a wrong guess at a label is worse than an exact key. */ -function toastWriteWarning(ev: WriteWarningEvent): void { - const fields = Array.from(new Set(ev.droppedFields.flatMap((d) => d.fields))); - if (fields.length === 0) return; - const list = fields.join(', '); - const description = - fields.length === 1 - ? `The read-only field “${list}” could not be changed and was not saved.` - : `${fields.length} read-only fields could not be changed and were not saved: ${list}.`; - toast.warning('Some fields were not saved', { description }); +function toastWriteWarning(ev: WriteWarningEvent, t: TranslateFn): void { + const byReason = new Map(); + for (const d of ev.droppedFields) { + const seen = byReason.get(d.reason) ?? []; + for (const f of d.fields) if (!seen.includes(f)) seen.push(f); + byReason.set(d.reason, seen); + } + const lines: string[] = []; + for (const [reason, fields] of byReason) { + if (fields.length === 0) continue; + const list = fields.join(', '); + lines.push( + reason === 'readonly_when' + ? t('detail.writeStrippedByState', { + fields: list, + defaultValue: + 'Not editable in this record\'s current state, so it was not saved: {{fields}}', + }) + : t('detail.writeStrippedReadonly', { + fields: list, + defaultValue: 'Read-only, so it was not saved: {{fields}}', + }), + ); + } + if (lines.length === 0) return; + toast.warning( + t('detail.writeStrippedTitle', { defaultValue: 'Some fields were not saved' }), + { description: lines.join('\n') }, + ); } interface AdapterProviderProps { @@ -44,6 +74,13 @@ interface AdapterProviderProps { */ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterProviderProps) { const [adapter, setAdapter] = useState(externalAdapter ?? null); + // The warning listener is registered ONCE (the adapter outlives a language + // switch), so read `t` through a ref instead of capturing it — otherwise a + // user who switches language mid-session keeps getting the old locale, and + // adding `t` to the effect deps would tear down and rebuild the adapter. + const { t } = useObjectTranslation(); + const tRef = useRef(t); + tRef.current = t; useEffect(() => { if (externalAdapter) { @@ -72,7 +109,9 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP // Surface silently-stripped write fields (#3431/#3455) as a toast so a // read-only value the user typed doesn't just vanish on save. - unsubscribeWriteWarning = a.onWriteWarning(toastWriteWarning); + unsubscribeWriteWarning = a.onWriteWarning((ev) => + toastWriteWarning(ev, tRef.current as TranslateFn), + ); await a.connect(); diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index 1a53f4ec3a..598a98b138 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -868,15 +868,31 @@ 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 = + // "An approval is in flight" — the record's own `approval_status` mirror OR + // an open pending request from the approvals API (the latter catches backends + // that track approvals without materializing the field). + const approvalPending = (pageRecord as any)?.approval_status === 'pending' || (pageRecord as any)?.approval_status === 'in_approval' || !!approvals.pendingRequest; + // …and, separately, "the record is LOCKED for writes" (#3794). These are not + // the same statement: an approval node with `lockRecord: false` leaves the + // record editable on purpose — that is how an approver amends a record while + // deciding on it — and the server's lock hook honors it (`lockRecord === false` + // ⇒ the update goes through). Conflating them made the console claim "Locked + // for approval" on a record it would happily save, so approvers never tried. + // + // The pending REQUEST is authoritative when we have one: `locks_record` comes + // from the same node-config snapshot the server hook reads (framework #3794). + // With no request in hand — approvals plugin absent, still loading, or a + // field-only backend — fall back to the `approval_status` mirror and assume + // locked, which is the safe direction (worst case we hide an edit affordance + // instead of letting a user fill a form the server will reject). + const approvalLocked = approvals.pendingRequest + ? approvals.pendingRequest.locks_record !== false + : approvalPending; + const approvalHandler = useCallback(async (action: ActionDef) => { const target = action.target || action.name; const params = (action.params && !Array.isArray(action.params)) @@ -1796,6 +1812,14 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // can't be stacked on top of the draft (two competing edit sessions // with no reconciliation). disableDuringInlineEdit: true, + // #3794 — an approval-LOCKED record refuses every write (the server + // answers RECORD_LOCKED), so opening the form only to be rejected on + // Save after filling a screen is wasted work. Disabled rather than + // hidden: the user should see the affordance exists and is off, with + // the lock band next to it saying why. Note this is the LOCK, not the + // mere presence of an approval — a `lockRecord: false` node keeps Edit + // live, which is the point of that setting. + disabled: approvalLocked, onClick: () => onEdit({ id: pureRecordId }), } as any); } @@ -1919,10 +1943,15 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri 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). */} + never materialize an `approval_status` field (objectui#2618). + `approvalPending` is the separate "a request is in flight" signal + (#3794): on a `lockRecord: false` node it is true while `locked` is + false, and the band says "in approval (editable)" instead of lying + about a lock the server does not enforce. */} implements DataSource { this.emitWriteWarning({ operation, resource, ...(id !== undefined ? { id } : {}), droppedFields }); } + /** + * Same, for the cross-object transactional batch (framework #3794). Its + * response hangs the events off a top-level `droppedFields` list, each tagged + * with the `index` of the operation it came from — `results` entries are bare + * record echoes with nowhere to hang a per-row list. + * + * This is the path that matters most for the warning: `batchTransaction` is + * how the console's record form saves a master-detail record, so a + * `readonlyWhen`-locked field edited in that form was stripped server-side + * while the UI reported a plain success. The operation kind is taken from the + * originating op so the toast doesn't call an update a create. + */ + private notifyBatchDroppedFields( + operations: BatchTransactionOperation[], + payload: unknown, + ): void { + const dropped = (payload as { droppedFields?: unknown } | null | undefined)?.droppedFields; + if (!Array.isArray(dropped) || dropped.length === 0) return; + for (const entry of dropped) { + if (!entry || typeof entry !== 'object') continue; + const e = entry as DroppedFieldsEvent & { index?: number }; + if (!Array.isArray(e.fields) || e.fields.length === 0) continue; + const op = typeof e.index === 'number' ? operations[e.index] : undefined; + // `delete` never drops fields; anything unexpected reads as an update, + // which is the truthful default for a batch that echoed a strip. + const operation: 'create' | 'update' = (op?.action ?? 'create') === 'create' ? 'create' : 'update'; + this.emitWriteWarning({ + operation, + resource: e.object ?? op?.object ?? '', + ...(op?.id !== undefined && op?.id !== null ? { id: op.id } : {}), + droppedFields: [{ object: e.object, fields: e.fields, reason: e.reason }], + }); + } + } + /** * Subscribe to write-warning events (a create/update dropped caller-supplied * fields — #3431/#3455). Returns an unsubscribe function. The app shell uses @@ -1225,6 +1260,7 @@ export class ObjectStackAdapter implements DataSource { // dependency floor (framework #3271). No hand-rolled POST /api/v1/batch. const payload = await this.client.data.batchTransaction(operations); this.emitBatchMutations(operations, payload?.results); + this.notifyBatchDroppedFields(operations, payload); return payload; } catch (err) { // On a non-declaring backend, endpoint missing (404/405) or a runtime that diff --git a/packages/data-objectstack/src/onWriteWarning.test.ts b/packages/data-objectstack/src/onWriteWarning.test.ts index 6571213d43..01e9a36101 100644 --- a/packages/data-objectstack/src/onWriteWarning.test.ts +++ b/packages/data-objectstack/src/onWriteWarning.test.ts @@ -94,6 +94,56 @@ describe('ObjectStackAdapter.onWriteWarning', () => { expect(events).toEqual([]); }); + // ── cross-object batch (framework #3794) ───────────────────────────────── + // + // `batchTransaction` is the console record form's save path for a + // master-detail record, so this is where a `readonlyWhen` strip actually + // reaches a user editing a form. Its response tags each event with the index + // of the operation that produced it. + + it('emits a write-warning per event on a cross-object batch, resolving the op', async () => { + const batchTransaction = vi.fn().mockResolvedValue({ + results: [{ id: 'acc1' }, { id: 'inv1' }], + droppedFields: [ + { object: 'invoice', fields: ['tax_rate'], reason: 'readonly_when', index: 1 }, + ], + }); + const ds = makeDS({ batchTransaction }); + ds.atomicBatchCapability = true; + const events: WriteWarningEvent[] = []; + ds.onWriteWarning((e: WriteWarningEvent) => events.push(e)); + + await ds.batchTransaction([ + { object: 'account', action: 'create', data: { name: 'Acme' } }, + { object: 'invoice', action: 'update', id: 'inv1', data: { status: 'paid', tax_rate: 9 } }, + ]); + + expect(events).toEqual([ + { + operation: 'update', + resource: 'invoice', + id: 'inv1', + droppedFields: [{ object: 'invoice', fields: ['tax_rate'], reason: 'readonly_when' }], + }, + ]); + }); + + it('does NOT emit for a clean batch or a malformed droppedFields list', async () => { + const batchTransaction = vi + .fn() + .mockResolvedValueOnce({ results: [{ id: 'a' }] }) + .mockResolvedValueOnce({ results: [{ id: 'a' }], droppedFields: [{ object: 'x', fields: [], index: 0 }] }); + const ds = makeDS({ batchTransaction }); + ds.atomicBatchCapability = true; + const events: WriteWarningEvent[] = []; + ds.onWriteWarning((e: WriteWarningEvent) => events.push(e)); + + await ds.batchTransaction([{ object: 'x', action: 'create', data: {} }]); + await ds.batchTransaction([{ object: 'x', action: 'create', data: {} }]); + + expect(events).toEqual([]); + }); + it('stops delivering after unsubscribe', async () => { const create = vi.fn().mockResolvedValue({ record: { id: 'r1' }, droppedFields: ONE_DROP }); const ds = makeDS({ create }); diff --git a/packages/data-objectstack/src/userState.test.ts b/packages/data-objectstack/src/userState.test.ts index 53876be162..88890d8199 100644 --- a/packages/data-objectstack/src/userState.test.ts +++ b/packages/data-objectstack/src/userState.test.ts @@ -144,7 +144,6 @@ describe('createObjectStackUserStateAdapter', () => { user_id: 'u1', key: 'ui.favorites', value: [{ id: 'a' }], - updated_at: expect.any(String), }); }); @@ -165,11 +164,35 @@ describe('createObjectStackUserStateAdapter', () => { expect(ds.update).toHaveBeenCalledWith('sys_user_preference', 'row-99', { value: [{ id: 'x' }], - updated_at: expect.any(String), }); expect(ds.create).not.toHaveBeenCalled(); }); + // #3794 — `updated_at` is server-managed. A caller-supplied value is + // stripped (framework #2948) and reported back as a dropped field, which + // the console turns into a "Some fields were not saved" toast. Stamping it + // here fired that warning on every recents/favorites write, about a field + // no user ever touched — noise on the exact surface that is supposed to + // tell a user their edit did not land. + it('never sends the server-managed updated_at column', async () => { + const ds = mockDataSource({ + find: vi.fn().mockResolvedValue({ data: [] }) as any, + create: vi.fn().mockResolvedValue({ id: 'row-1' }) as any, + }); + const adapter = createObjectStackUserStateAdapter({ + dataSource: ds, + userId: 'u', + key: 'ui.recent', + }); + + await adapter.save([{ id: 'a' } as any]); + await adapter.save([{ id: 'b' } as any]); + + for (const call of [...(ds.create as any).mock.calls, ...(ds.update as any).mock.calls]) { + expect(call[call.length - 1]).not.toHaveProperty('updated_at'); + } + }); + it('uses the cached row id from a previous load on subsequent saves', async () => { const ds = mockDataSource({ find: vi.fn().mockResolvedValue({ @@ -242,7 +265,6 @@ describe('createObjectStackUserStateAdapter', () => { expect(ds.create).toHaveBeenCalledTimes(1); expect(ds.update).toHaveBeenCalledWith('sys_user_preference', 'row-7', { value: [{ id: 'q' }], - updated_at: expect.any(String), }); }); @@ -270,7 +292,6 @@ describe('createObjectStackUserStateAdapter', () => { expect(ds.create).toHaveBeenCalledTimes(1); expect(ds.update).toHaveBeenCalledWith('sys_user_preference', 'row-1', { value: [{ id: 'a' }, { id: 'b' }], - updated_at: expect.any(String), }); }); diff --git a/packages/data-objectstack/src/userState.ts b/packages/data-objectstack/src/userState.ts index c4f1c8d218..a4a6af4a62 100644 --- a/packages/data-objectstack/src/userState.ts +++ b/packages/data-objectstack/src/userState.ts @@ -159,13 +159,18 @@ export function createObjectStackUserStateAdapter( // (a concurrent writer created the row, or the backend dropped our find // predicate), recover by re-finding and updating rather than surfacing the // failed insert. + // + // Deliberately does NOT stamp `updated_at`: the column is server-managed, and + // a non-system caller's write to it is stripped (framework #2948) and reported + // back as a dropped field — which the console surfaces as a "Some fields were + // not saved" toast (#3431). Sending it made every recents/favorites write pop + // a scary warning about a field the user never touched, drowning the real + // signal the toast exists for (#3794). The server stamps it either way. const upsert = async (items: T[]): Promise => { - const now = new Date().toISOString(); - // Fast path: we already know the row id from a previous load/save. if (cachedRowId !== null) { try { - await dataSource.update(resource, cachedRowId, { value: items, updated_at: now }); + await dataSource.update(resource, cachedRowId, { value: items }); return; } catch (updateError) { // Row may have been deleted server-side — fall through to find/insert. @@ -177,7 +182,7 @@ export function createObjectStackUserStateAdapter( const existing = await findExisting(); if (existing && existing.id !== undefined && existing.id !== null) { cachedRowId = existing.id; - await dataSource.update(resource, existing.id, { value: items, updated_at: now }); + await dataSource.update(resource, existing.id, { value: items }); return; } @@ -186,7 +191,6 @@ export function createObjectStackUserStateAdapter( user_id: userId, key, value: items, - updated_at: now, }); const newId = (created as UserPreferenceRecord | undefined)?.id; if (newId !== undefined && newId !== null) cachedRowId = newId; @@ -196,7 +200,7 @@ export function createObjectStackUserStateAdapter( const recovered = await findExisting(); if (recovered && recovered.id !== undefined && recovered.id !== null) { cachedRowId = recovered.id; - await dataSource.update(resource, recovered.id, { value: items, updated_at: now }); + await dataSource.update(resource, recovered.id, { value: items }); return; } throw createError; diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index f874021eea..c3161c1d89 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -680,9 +680,15 @@ const en = { editInlineHint: 'Double-click to edit', lockedByApproval: 'Locked for approval', lockedTooltip: 'This record has a pending approval request; editing is locked', + writeStrippedTitle: 'Some fields were not saved', + writeStrippedReadonly: 'Read-only, so it was not saved: {{fields}}', + writeStrippedByState: "Not editable in this record's current state, so it was not saved: {{fields}}", + inApprovalEditable: 'In approval (editable)', + inApprovalEditableTooltip: 'This record has a pending approval request, but this step does not lock it — you can still edit', cancelApproval: 'Recall approval', cancelApprovalInFlight: 'Recalling…', cancelApprovalTooltip: 'Recall the pending approval request to unlock this record', + cancelApprovalTooltipUnlocked: 'Recall the pending approval request', cancelApprovalFailed: 'Failed to recall approval', cancelApprovalUnavailable: 'Recalling approvals is not supported on this data source', linkCopied: 'Link copied to clipboard', diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index f522a19402..e20a2f76b7 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -692,9 +692,15 @@ const zh = { editInlineHint: '双击编辑', lockedByApproval: '审批中已锁定', lockedTooltip: '该记录有待审批的请求，编辑已被锁定', + writeStrippedTitle: '部分字段未保存', + writeStrippedReadonly: '以下字段为只读，未保存：{{fields}}', + writeStrippedByState: '以下字段在记录当前状态下不可编辑，未保存：{{fields}}', + inApprovalEditable: '审批中（可编辑）', + inApprovalEditableTooltip: '该记录有待审批的请求，但当前审批节点未锁定记录，仍可编辑', cancelApproval: '撤回审批', cancelApprovalInFlight: '撤回中…', cancelApprovalTooltip: '撤回当前的待审批请求以解除记录锁定', + cancelApprovalTooltipUnlocked: '撤回当前的待审批请求', cancelApprovalFailed: '撤回审批失败', cancelApprovalUnavailable: '当前数据源不支持撤回审批', linkCopied: '链接已复制到剪贴板', diff --git a/packages/plugin-detail/src/DetailView.tsx b/packages/plugin-detail/src/DetailView.tsx index e81583221a..03f507fb07 100644 --- a/packages/plugin-detail/src/DetailView.tsx +++ b/packages/plugin-detail/src/DetailView.tsx @@ -31,6 +31,7 @@ import { ChevronRight, Copy, Lock, + Clock, X, } from 'lucide-react'; import { DetailSection } from './DetailSection'; @@ -1017,43 +1018,73 @@ export const DetailView: React.FC = ({ bar itself now lives in the record-level (objectui#2407 P1); this band is lock-only. */} {inlineEdit && schema.showHeader === false && (() => { - // Detect approval lock. Prefer the host-supplied signal - // (`inline.locked`) — the record-level session computes it from the - // record's `approval_status` field OR an open approvals request - // (objectui#2618), so the band engages even on backends that track - // the lock via approval requests only and never materialize an - // `approval_status` field on the record. Fall back to the record's - // own field for bare/legacy DetailView usage without a host that - // threads the lock. Either way a locked record's writes are rejected - // with RECORD_LOCKED, so surface the badge instead of editing and - // failing on save. + // Two DISTINCT states to surface (#3794): an approval is in flight, and + // — separately — the record is locked for writes. An approval node + // declaring `lockRecord: false` is pending WITHOUT locking (the server's + // lock hook returns early and the save lands), so painting the same + // "Locked for approval" band on both told the approver the opposite of + // what the backend does, and the node's whole point — amend the record + // while deciding — went unused. + // + // The host is authoritative when it has an opinion: the record-level + // session resolves both from the pending approval REQUEST, whose + // `locks_record` is the same node-config snapshot the server hook reads. + // Only when no host threads `approvalPending` (bare/legacy DetailView) + // do we fall back to the record's own `approval_status` field — and that + // mirror can only say "in approval", so the fallback assumes a lock, + // which is the safe direction and preserves the objectui#2618 behavior. const approvalStatus = data?.approval_status; - const isLocked = - (inline?.locked ?? false) || - approvalStatus === 'pending' || - approvalStatus === 'in_approval'; - // Nothing to surface (not locked, no approval-cancel error): no band. - if (!isLocked && !saveError) return null; + const fieldPending = + approvalStatus === 'pending' || approvalStatus === 'in_approval'; + const hostKnows = inline?.approvalPending !== undefined; + const isLocked = hostKnows + ? (inline?.locked ?? false) + : ((inline?.locked ?? false) || fieldPending); + // A lock always implies an in-flight approval, so a host that threads + // only `locked` (objectui#2618, before `approvalPending` existed) keeps + // its band. + const isPending = isLocked || (hostKnows ? !!inline?.approvalPending : fieldPending); + // Nothing to surface (no approval in flight, no approval-cancel error). + if (!isPending && !saveError) return null; return (
- {isLocked && ( + {isPending && (
- - - {t('detail.lockedByApproval')} - + {isLocked ? ( + + + {t('detail.lockedByApproval')} + + ) : ( + // Pending but writable — a neutral (not amber-warning) band, and + // no lock glyph: nothing here is blocked. The recall affordance + // stays, since recalling is about the request, not the lock. + + + {t('detail.inApprovalEditable')} + + )} {dataSource?.cancelPendingApproval && (