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..7f5aa3350e --- /dev/null +++ b/.changeset/approval-band-truth-and-write-warnings.md @@ -0,0 +1,53 @@ +--- +"@object-ui/plugin-detail": minor +"@object-ui/app-shell": minor +"@object-ui/data-objectstack": minor +"@object-ui/i18n": minor +--- + +fix(detail): finish the approval-lock story, 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". + +**The lock band told the truth; the Edit button did not.** objectui#2902 split +the band into "in approval · editable" vs locked, but the header **Edit** CTA +still keyed off nothing at all — on a genuinely locked record it stayed live, so +the user opened the form, filled a screen, and got `RECORD_LOCKED` back on Save. +It is now `disabled` on a locked record: visible-but-off, with the band beside it +saying why. 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. + +**And the band could still re-lock itself.** `DetailView` OR-ed the record's own +`approval_status` mirror into `isLocked` unconditionally. That mirror is written +on submit by any flow configuring an `approvalStatusField`, *regardless of* +`lockRecord` — so on a `lockRecord: false` node the host correctly resolved "not +locked" from the request's `lock_record` while the mirror dragged the band back +to "Locked for approval", with the pencils live and saves landing underneath it. +The host is now authoritative whenever it threads `approvalPending`; the mirror +is consulted only for bare/legacy `DetailView` hosts that thread nothing, where +it still reads as locked (no node granularity — the safe direction). + +Recall's tooltip no longer promises to unlock a record the node never locked +(`detail.cancelApprovalTooltipUnlocked`). + +**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*`, ten locales) 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, so "read-only" sent the user hunting for a +permission problem that does not exist. + +**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/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 f295123914..4f21a44096 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -1839,6 +1839,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, + // framework#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); } diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 5643dc75d6..f9c548132c 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -973,6 +973,41 @@ export class ObjectStackAdapter 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/ar.ts b/packages/i18n/src/locales/ar.ts index 5d202e5a21..21accb5343 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -792,11 +792,15 @@ const ar = { saving: "جارٍ الحفظ…", lockedByApproval: "مقفل للموافقة", lockedTooltip: "يحتوي هذا السجل على طلب موافقة معلق؛ التعديل مقفل", + writeStrippedTitle: "لم يتم حفظ بعض الحقول", + writeStrippedReadonly: "للقراءة فقط، لذلك لم يتم حفظها: {{fields}}", + writeStrippedByState: "غير قابلة للتعديل في الحالة الحالية لهذا السجل، لذلك لم يتم حفظها: {{fields}}", approvalPendingEditable: "قيد الموافقة · قابل للتعديل", approvalPendingTooltip: "يحتوي هذا السجل على طلب موافقة معلق، لكن هذه الخطوة لا تزال تسمح بالتعديل", cancelApproval: "إلغاء الموافقة", cancelApprovalInFlight: "جارٍ الإلغاء…", cancelApprovalTooltip: "إلغاء طلب الموافقة المعلق لفتح قفل السجل", + cancelApprovalTooltipUnlocked: "إلغاء طلب الموافقة المعلق", cancelApprovalFailed: "فشل إلغاء الموافقة", cancelApprovalUnavailable: "إلغاء الموافقات غير مدعوم من مصدر البيانات هذا", linkCopied: "تم نسخ الرابط إلى الحافظة", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 1abdc5c05a..c2227f0f6a 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -790,11 +790,15 @@ const de = { saving: "Speichern…", lockedByApproval: "Zur Genehmigung gesperrt", lockedTooltip: "Dieser Datensatz hat eine ausstehende Genehmigungsanfrage; die Bearbeitung ist gesperrt", + writeStrippedTitle: "Einige Felder wurden nicht gespeichert", + writeStrippedReadonly: "Schreibgeschützt und daher nicht gespeichert: {{fields}}", + writeStrippedByState: "Im aktuellen Status dieses Datensatzes nicht bearbeitbar und daher nicht gespeichert: {{fields}}", 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", + cancelApprovalTooltipUnlocked: "Ausstehende Genehmigungsanfrage zurückziehen", cancelApprovalFailed: "Genehmigung konnte nicht zurückgezogen werden", cancelApprovalUnavailable: "Das Zurückziehen von Genehmigungen wird bei dieser Datenquelle nicht unterstützt", linkCopied: "Link in die Zwischenablage kopiert", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index 4df5cae437..843f2c1f95 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -708,11 +708,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}}", 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', + 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/es.ts b/packages/i18n/src/locales/es.ts index 33c6d211b3..3499224b5e 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -790,11 +790,15 @@ const es = { saving: "Guardando…", lockedByApproval: "Bloqueado para aprobación", lockedTooltip: "Este registro tiene una solicitud de aprobación pendiente; la edición está bloqueada", + writeStrippedTitle: "Algunos campos no se guardaron", + writeStrippedReadonly: "De solo lectura, por lo que no se guardó: {{fields}}", + writeStrippedByState: "No editable en el estado actual de este registro, por lo que no se guardó: {{fields}}", 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", + cancelApprovalTooltipUnlocked: "Cancelar la solicitud de aprobación pendiente", cancelApprovalFailed: "No se pudo cancelar la aprobación", cancelApprovalUnavailable: "La cancelación de aprobaciones no es compatible con esta fuente de datos", linkCopied: "Enlace copiado al portapapeles", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index a07cba6b2e..7f118d29cd 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -792,11 +792,15 @@ const fr = { saving: "Enregistrement…", lockedByApproval: "Verrouillé pour approbation", lockedTooltip: "Cet enregistrement a une demande d'approbation en attente ; la modification est verrouillée", + writeStrippedTitle: "Certains champs n'ont pas été enregistrés", + writeStrippedReadonly: "En lecture seule, donc non enregistré : {{fields}}", + writeStrippedByState: "Non modifiable dans l'état actuel de cet enregistrement, donc non enregistré : {{fields}}", 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", + cancelApprovalTooltipUnlocked: "Annuler la demande d'approbation en attente", cancelApprovalFailed: "Échec de l'annulation de l'approbation", cancelApprovalUnavailable: "L'annulation des approbations n'est pas prise en charge par cette source de données", linkCopied: "Lien copié dans le presse-papiers", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 9243e9ec18..98f990456e 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -801,11 +801,15 @@ const ja = { saving: "保存中…", lockedByApproval: "承認のためロック中", lockedTooltip: "このレコードには承認待ちのリクエストがあります。編集はロックされています", + writeStrippedTitle: "保存されなかった項目があります", + writeStrippedReadonly: "次の項目は読み取り専用のため保存されませんでした: {{fields}}", + writeStrippedByState: "次の項目はこのレコードの現在の状態では編集できないため保存されませんでした: {{fields}}", approvalPendingEditable: "承認中 · 編集可能", approvalPendingTooltip: "このレコードには承認待ちのリクエストがありますが、このステップでは編集できます", cancelApproval: "承認を取り消す", cancelApprovalInFlight: "取り消し中…", cancelApprovalTooltip: "承認待ちリクエストを取り消してレコードのロックを解除する", + cancelApprovalTooltipUnlocked: "承認待ちリクエストを取り消す", cancelApprovalFailed: "承認の取り消しに失敗しました", cancelApprovalUnavailable: "このデータソースでは承認の取り消しはサポートされていません", linkCopied: "リンクをクリップボードにコピーしました", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 1808527373..819ad41e71 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -790,11 +790,15 @@ const ko = { saving: "저장 중…", lockedByApproval: "승인을 위해 잠김", lockedTooltip: "이 레코드에 대기 중인 승인 요청이 있습니다. 편집이 잠겨 있습니다", + writeStrippedTitle: "일부 필드가 저장되지 않았습니다", + writeStrippedReadonly: "다음 필드는 읽기 전용이므로 저장되지 않았습니다: {{fields}}", + writeStrippedByState: "다음 필드는 이 레코드의 현재 상태에서 편집할 수 없으므로 저장되지 않았습니다: {{fields}}", approvalPendingEditable: "승인 진행 중 · 편집 가능", approvalPendingTooltip: "이 레코드에 대기 중인 승인 요청이 있지만 이 단계에서는 편집할 수 있습니다", cancelApproval: "승인 취소", cancelApprovalInFlight: "취소 중…", cancelApprovalTooltip: "대기 중인 승인 요청을 취소하여 레코드 잠금 해제", + cancelApprovalTooltipUnlocked: "대기 중인 승인 요청 취소", cancelApprovalFailed: "승인 취소 실패", cancelApprovalUnavailable: "이 데이터 소스에서는 승인 취소가 지원되지 않습니다", linkCopied: "링크가 클립보드에 복사됨", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index a4dba539a9..6e21f67bed 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -792,11 +792,15 @@ 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", + writeStrippedTitle: "Alguns campos não foram salvos", + writeStrippedReadonly: "Somente leitura, portanto não foi salvo: {{fields}}", + writeStrippedByState: "Não editável no estado atual deste registro, portanto não foi salvo: {{fields}}", 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", + cancelApprovalTooltipUnlocked: "Cancelar a solicitação de aprovação pendente", cancelApprovalFailed: "Falha ao cancelar aprovação", cancelApprovalUnavailable: "O cancelamento de aprovações não é suportado por esta fonte de dados", linkCopied: "Link copiado para a área de transferência", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index e29d61d136..d4490ca96e 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -803,11 +803,15 @@ const ru = { saving: "Сохранение…", lockedByApproval: "Заблокировано для согласования", lockedTooltip: "У этой записи есть ожидающий запрос на согласование; редактирование заблокировано", + writeStrippedTitle: "Часть полей не сохранена", + writeStrippedReadonly: "Поля только для чтения, поэтому не сохранены: {{fields}}", + writeStrippedByState: "Поля недоступны для редактирования в текущем состоянии записи, поэтому не сохранены: {{fields}}", approvalPendingEditable: "На согласовании · редактирование доступно", approvalPendingTooltip: "У этой записи есть ожидающий запрос на согласование, но этот шаг всё ещё разрешает редактирование", cancelApproval: "Отменить согласование", cancelApprovalInFlight: "Отмена…", cancelApprovalTooltip: "Отмените ожидающий запрос на согласование, чтобы разблокировать запись", + cancelApprovalTooltipUnlocked: "Отменить ожидающий запрос на согласование", cancelApprovalFailed: "Не удалось отменить согласование", cancelApprovalUnavailable: "Отмена согласований не поддерживается этим источником данных", linkCopied: "Ссылка скопирована в буфер обмена", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index e38e3de54b..554e872cb9 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -720,11 +720,15 @@ const zh = { editInlineHint: '双击编辑', lockedByApproval: '审批中已锁定', lockedTooltip: '该记录有待审批的请求,编辑已被锁定', + writeStrippedTitle: '部分字段未保存', + writeStrippedReadonly: '以下字段为只读,未保存:{{fields}}', + writeStrippedByState: '以下字段在记录当前状态下不可编辑,未保存:{{fields}}', approvalPendingEditable: '审批中 · 可编辑', approvalPendingTooltip: '该记录有待审批的请求,但当前审批节点仍允许编辑', cancelApproval: '撤回审批', cancelApprovalInFlight: '撤回中…', cancelApprovalTooltip: '撤回当前的待审批请求以解除记录锁定', + cancelApprovalTooltipUnlocked: '撤回当前的待审批请求', cancelApprovalFailed: '撤回审批失败', cancelApprovalUnavailable: '当前数据源不支持撤回审批', linkCopied: '链接已复制到剪贴板', diff --git a/packages/plugin-detail/src/DetailView.tsx b/packages/plugin-detail/src/DetailView.tsx index f6634656a7..0f883bdf3f 100644 --- a/packages/plugin-detail/src/DetailView.tsx +++ b/packages/plugin-detail/src/DetailView.tsx @@ -1073,18 +1073,39 @@ export const DetailView: React.FC = ({ // the user "locked" on a record they could freely edit, and the // approver — the very person the flag exists to let edit — never tried. // - // Prefer the host-supplied signals: the record-level session resolves - // the pending node's policy from the approvals API (objectui#2618 for - // why the record field alone is not enough). The record's own - // `approval_status` remains the fallback for bare/legacy DetailView - // usage with no host threading the state; it carries no node + // The record's own `approval_status` is the fallback for bare/legacy + // DetailView usage where no host threads the state (objectui#2618 for + // why the record field alone is not enough). It carries no node // granularity, so it can only mean "locked" — the safe read, since a // wrongly-offered edit dies on the server with RECORD_LOCKED. + // + // But it must NOT be OR-ed in unconditionally, because the two sources + // genuinely disagree in a shipping configuration: a flow configuring an + // `approvalStatusField` mirrors `approval_status: 'pending'` onto the + // record on submit *regardless of* `lockRecord` (framework + // `mirrorStatusField`), so the mirror would drag the band back to + // "Locked for approval" on exactly the `lockRecord: false` node this + // feature exists to free — pencils live and saves landing underneath it. + // + // `approvalPending && !locked` is the tell that the host has an actual + // opinion. `InlineEditProvider` defaults `approvalPending` to `locked`, + // so a host threading only `locked` (pre-#2902) always reports the two + // equal and can never produce that combination; a host that resolved + // the pending node's `lock_record` is the only thing that can. When it + // speaks, it wins — it read the same snapshot the server's lock hook + // enforces. const approvalStatus = data?.approval_status; const statusPending = approvalStatus === 'pending' || approvalStatus === 'in_approval'; - const isLocked = (inline?.locked ?? false) || statusPending; - const isPending = (inline?.approvalPending ?? false) || isLocked; + const hostPending = inline?.approvalPending ?? false; + const hostSaysEditable = hostPending && !(inline?.locked ?? false); + const isLocked = hostSaysEditable + ? false + : ((inline?.locked ?? false) || statusPending); + // 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 || hostPending || statusPending; // Nothing to surface (no approval, no approval-cancel error): no band. if (!isPending && !saveError) return null; return ( @@ -1126,7 +1147,12 @@ export const DetailView: React.FC = ({ ? 'gap-2 border-amber-300 text-amber-800 hover:bg-amber-50' : 'gap-2 border-sky-300 text-sky-800 hover:bg-sky-50' } - title={t('detail.cancelApprovalTooltip')} + // The locked variant's tooltip promises to UNLOCK the record; + // on a node that never locked it, that sentence describes an + // effect the click does not have. + title={isLocked + ? t('detail.cancelApprovalTooltip') + : t('detail.cancelApprovalTooltipUnlocked')} > diff --git a/packages/plugin-detail/src/__tests__/DetailView.approvalBand.test.tsx b/packages/plugin-detail/src/__tests__/DetailView.approvalBand.test.tsx index 1d8e520a51..b5740740cc 100644 --- a/packages/plugin-detail/src/__tests__/DetailView.approvalBand.test.tsx +++ b/packages/plugin-detail/src/__tests__/DetailView.approvalBand.test.tsx @@ -114,4 +114,19 @@ describe('DetailView – approval band, editable vs locked (objectui#2902)', () renderBand({ locked: false, approvalPending: false }, { approval_status: 'draft' }); expect(screen.queryByRole('status')).not.toBeInTheDocument(); }); + + it('lets the host verdict beat the approval_status mirror on an unlocked node', () => { + // The configuration where the two sources genuinely disagree: a flow with + // an `approvalStatusField` mirrors `approval_status: 'pending'` onto the + // record on submit no matter what `lockRecord` says, so a `lockRecord: + // false` node has BOTH the mirror reading "pending" and a host verdict of + // "not locked". OR-ing the mirror in would re-lock the band on exactly the + // node this feature exists to free — pencils live and saves landing under + // a band that says "Locked for approval". The host resolved its verdict + // from the request's `lock_record`, which is the same snapshot the + // server's lock hook reads, so it wins. + renderBand({ locked: false, approvalPending: true }, { approval_status: 'pending' }); + expect(screen.getByText('In approval · editable')).toBeInTheDocument(); + expect(screen.queryByText('Locked for approval')).not.toBeInTheDocument(); + }); }); diff --git a/packages/plugin-detail/src/useDetailTranslation.ts b/packages/plugin-detail/src/useDetailTranslation.ts index 85fa71d4f2..4ce9d1d512 100644 --- a/packages/plugin-detail/src/useDetailTranslation.ts +++ b/packages/plugin-detail/src/useDetailTranslation.ts @@ -172,11 +172,14 @@ export const DETAIL_DEFAULT_TRANSLATIONS: Record = { // Approval band (objectui#2618; two-state since #2902) 'detail.lockedByApproval': 'Locked for approval', 'detail.lockedTooltip': 'This record has a pending approval request; editing is locked', + // …and the pending-but-writable variant: the approval node declares + // `lockRecord: false`, so the record stays editable while the request is open. 'detail.approvalPendingEditable': 'In approval · editable', 'detail.approvalPendingTooltip': 'This record has a pending approval request; this step still allows editing', 'detail.cancelApproval': 'Recall approval', 'detail.cancelApprovalInFlight': 'Recalling…', 'detail.cancelApprovalTooltip': 'Recall the pending approval request to unlock this record', + 'detail.cancelApprovalTooltipUnlocked': 'Recall the pending approval request', 'detail.cancelApprovalFailed': 'Failed to recall approval', 'detail.cancelApprovalUnavailable': 'Recalling approvals is not supported on this data source', };