diff --git a/.changeset/6464-recall-submitter-gate.md b/.changeset/6464-recall-submitter-gate.md new file mode 100644 index 0000000000..541e31c934 --- /dev/null +++ b/.changeset/6464-recall-submitter-gate.md @@ -0,0 +1,61 @@ +--- +'@object-ui/react': minor +'@object-ui/plugin-detail': patch +'@object-ui/app-shell': patch +--- + +The record page's approval band offers its **Recall** button to the approval's submitter +only (objectui#6464). + +Field report on `@objectstack/*@17.2.0`: user A submits a record into a 4-level approval; +user B — not the submitter, read access, not an admin — opens the record and the band still +lights a clickable recall button. The click cannot succeed. The recall endpoint authorizes +on submitter identity and refuses everyone else, so the only outcome available to that +button was a failure toast. Record state was never at risk; this was purely a +writability-feedback mismatch, the same family as objectui#3794. + +The button's only gate was `dataSource.cancelPendingApproval` — "can this adapter recall at +all" — which is a question about the DataSource, not about the viewer. Identity now joins +it, threaded the way every other signal on that band already travels: the HOST resolves it +and passes it through `InlineEditProvider`, so the renderer stays DataSource-agnostic and +never re-derives who submitted what. + +- `@object-ui/react` — `InlineEditProvider` accepts `approvalIsSubmitter`, surfaced on + `InlineEditContextValue`. Additive and optional; no existing prop changes. +- `@object-ui/plugin-detail` — the band's recall button is withdrawn when that signal is a + resolved `false`. +- `@object-ui/app-shell` — `RecordDetailView` resolves the verdict from its existing + approvals read and threads it. + +**The signal is tri-state, and the third state is the load-bearing one.** `true` offers +recall, `false` withdraws it, and **`undefined` — a host that resolves no approval identity +— renders exactly as it did before this release.** Omission preserving prior behaviour +mirrors how `approvalPending` falls back to `locked`. Defaulting the unknown case to "hide" +would have traded a cosmetic defect for a functional loss: every host whose band runs off +the record's `approval_status` mirror alone would silently lose its submitter's only way to +unlock their own record. + +**Withdrawn rather than disabled-with-reason.** The card offered either. For a +non-submitter this control is never actionable on any pending record, so a permanently +disabled button is standing clutter rather than a lesson; and the two sibling submitter +levers already hide — the approvals panel's Remind button, and the declared +`approval_recall` action's `visible` predicate. The band, its quorum tally and the +approvals timeline still tell a non-submitter exactly what state the record is in. Only the +lever they can never pull is gone. + +**This changes no permission.** Nothing about what the server allows moves, `canEdit` and +the approval lock are untouched, and nothing downstream reads `approvalIsSubmitter` as an +authorization verdict — the recall endpoint remains the sole authority, and it refused +these callers before this change and refuses them after. There is deliberately **no admin +carve-out** (the reporter ruled that case out, cf. objectstack#9464). + +The derivation itself is now one function, `isSubmitterOf` — server-resolved +`viewer.is_submitter` first (framework#3310), an id comparison as the fallback for backends +that predate it, joined with `??` so a server that resolved `false` is believed rather than +re-litigated client-side. The approvals panel's Remind gate, which already carried that +expression inline and whose behaviour is unchanged, now reads the same answer: two copies +would have been two definitions of who submitted. + +The **untranslated refusal text** the reporter also saw ("No pending approval request found +for this record", concatenated after a localized prefix) is a separate defect and is not +addressed here; it is tracked on objectstack#11993. diff --git a/packages/app-shell/src/hooks/__tests__/useRecordApprovals.isSubmitterOf.test.ts b/packages/app-shell/src/hooks/__tests__/useRecordApprovals.isSubmitterOf.test.ts new file mode 100644 index 0000000000..915aaab94f --- /dev/null +++ b/packages/app-shell/src/hooks/__tests__/useRecordApprovals.isSubmitterOf.test.ts @@ -0,0 +1,103 @@ +/** + * 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 { isSubmitterOf, type ApprovalRequestLite } from '../useRecordApprovals'; + +/** + * `isSubmitterOf` — the ONE derivation of "did this viewer submit this + * approval" (objectui#6464). + * + * Two surfaces gate on the answer: the approvals panel's Remind button and the + * record band's Recall button. Both are server-authorized on submitter + * identity, so two client-side copies of the derivation would be two + * definitions of who submitted — the drift `utils/approverIdentity` already + * exists to prevent on the display side. + * + * What the cells below actually protect is the SOURCE ORDER. The server's + * `viewer.is_submitter` (framework#3310) is the resolution the endpoint will + * enforce; the id comparison exists only for backends that predate the `viewer` + * block. Joining them with `||` instead of `??` would look identical on every + * agreeing row and quietly overturn the server on the one row where it says no. + */ + +const row = (over: Partial = {}): ApprovalRequestLite => ({ + id: 'req_1', + process_name: 'flow:budget_review', + object_name: 'budget', + record_id: 'B1', + status: 'pending', + submitter_id: 'u_alice', + ...over, +}); + +describe('isSubmitterOf — server-resolved first (objectui#6464)', () => { + it('believes the server when it says the viewer IS the submitter', () => { + expect(isSubmitterOf(row({ viewer: { can_act: false, is_submitter: true } }), 'u_bob')).toBe(true); + }); + + it('believes the server when it says the viewer is NOT the submitter', () => { + expect(isSubmitterOf(row({ viewer: { can_act: false, is_submitter: false } }), 'u_bob')).toBe(false); + }); + + /** + * The cell that separates `??` from `||`. The server resolved `false` while + * the raw `submitter_id` matches the signed-in id — a real shape whenever the + * server's answer accounts for something the bare column does not (a + * delegated or system-rewritten submission). `||` would re-litigate the + * server's refusal client-side and return `true`, lighting a lever the recall + * endpoint then rejects: the very defect this gate exists to close. + */ + it('does not let a matching submitter_id overturn a server `false`', () => { + expect( + isSubmitterOf(row({ submitter_id: 'u_alice', viewer: { can_act: false, is_submitter: false } }), 'u_alice'), + ).toBe(false); + }); +}); + +describe('isSubmitterOf — the id fallback for pre-`viewer` backends', () => { + it('matches the submitter by id when the server sends no viewer block', () => { + expect(isSubmitterOf(row({ submitter_id: 'u_alice' }), 'u_alice')).toBe(true); + }); + + it('rejects a non-submitter by id when the server sends no viewer block', () => { + expect(isSubmitterOf(row({ submitter_id: 'u_alice' }), 'u_bob')).toBe(false); + }); + + /** + * Fallback-also-absent: an older server AND no signed-in id to compare + * against. This resolves to a definite `false`, NOT to "unknown" — the + * authoritative row is in hand and nothing in it identifies this viewer as + * the submitter. It is also the same answer the Remind gate has always given + * in this shape, which is the point of there being one derivation. + */ + it('resolves to false when there is no id to compare against', () => { + expect(isSubmitterOf(row({ submitter_id: 'u_alice' }), undefined)).toBe(false); + expect(isSubmitterOf(row({ submitter_id: 'u_alice' }), null)).toBe(false); + expect(isSubmitterOf(row({ submitter_id: 'u_alice' }), '')).toBe(false); + }); + + it('resolves to false when the row itself names no submitter', () => { + // Both sides absent must not collapse into `undefined === undefined`. + expect(isSubmitterOf(row({ submitter_id: null }), undefined)).toBe(false); + expect(isSubmitterOf(row({ submitter_id: undefined }), undefined)).toBe(false); + }); +}); + +describe('isSubmitterOf — "unknown" is its own answer', () => { + /** + * No request to consult is NOT a denial. Callers feeding a feedback gate keep + * their prior behaviour on `undefined`; collapsing it to `false` here would + * hide the affordance on every backend that exposes no approvals API at all. + * Asserted with `toBeUndefined`, since `toBeFalsy` passes for both. + */ + it('returns undefined — not false — with no request', () => { + expect(isSubmitterOf(null, 'u_alice')).toBeUndefined(); + expect(isSubmitterOf(undefined, 'u_alice')).toBeUndefined(); + }); +}); diff --git a/packages/app-shell/src/hooks/useRecordApprovals.ts b/packages/app-shell/src/hooks/useRecordApprovals.ts index 3e02a82ae8..725ba6bcbc 100644 --- a/packages/app-shell/src/hooks/useRecordApprovals.ts +++ b/packages/app-shell/src/hooks/useRecordApprovals.ts @@ -199,6 +199,43 @@ export function recordLockedByApproval(request: ApprovalRequestLite | null | und return request.lock_record !== false; } +/** + * Did the current viewer submit this approval request? + * + * The submitter's levers — Remind on the approvals panel, Recall on the record + * band (objectui#6464) — are the ones the server authorizes on submitter + * identity and refuses to anyone else. This is the ONE place that answer is + * derived, so the two surfaces cannot drift into disagreeing about who the + * submitter is (the identical hazard `utils/approverIdentity` exists for on the + * display side). + * + * Source order, and it matters: the server-resolved `viewer.is_submitter` + * (framework#3310) wins, because the server already did the resolution the + * recall endpoint will enforce. The id comparison is the FALLBACK for backends + * that predate the `viewer` block — `??` and not `||`, so a server that + * resolved `false` is believed rather than being re-litigated client-side. + * + * Returns `undefined` — "unknown", not "no" — when there is no request to + * consult. A caller feeding a feedback gate must keep its prior behaviour on + * `undefined` rather than reading it as a denial; a caller that needs a plain + * boolean coerces explicitly. With a request in hand, an unresolvable viewer is + * a definite `false`: the authoritative row is present and nothing in it + * identifies this viewer as the submitter. + * + * ⚠️ FEEDBACK ONLY — this decides which affordances are SHOWN, never who MAY + * act. Every lever it gates is authorized server-side independently. + */ +export function isSubmitterOf( + request: ApprovalRequestLite | null | undefined, + currentUserId: string | null | undefined, +): boolean | undefined { + if (!request) return undefined; + return ( + request.viewer?.is_submitter + ?? (!!currentUserId && request.submitter_id === currentUserId) + ); +} + interface UseRecordApprovalsResult { loading: boolean; available: boolean; diff --git a/packages/app-shell/src/views/RecordApprovalsPanel.tsx b/packages/app-shell/src/views/RecordApprovalsPanel.tsx index 1b011c493d..7692662c2e 100644 --- a/packages/app-shell/src/views/RecordApprovalsPanel.tsx +++ b/packages/app-shell/src/views/RecordApprovalsPanel.tsx @@ -13,6 +13,7 @@ import { toast } from 'sonner'; import { createAuthenticatedFetch } from '@object-ui/auth'; import { useObjectTranslation } from '@object-ui/react'; import { + isSubmitterOf, listApprovalActions, remindApprovalRequest, type ApprovalActionLite, @@ -268,11 +269,14 @@ export const RecordApprovalsPanel: React.FC = ({ * Remind is the submitter's lever (server-authorized): prefer the * server-resolved `viewer.is_submitter` from the enriched pending row and * fall back to a plain id match for backends predating framework#3310. + * + * The derivation moved to `isSubmitterOf` unchanged (objectui#6464) so the + * record band's Recall gate reads the SAME answer — two copies of it would be + * two definitions of who submitted. `?? false` restates this call site's own + * reading of "no pending request": there is nothing to remind about, so the + * button is gone either way. */ - const isSubmitter = pendingRequest - ? pendingRequest.viewer?.is_submitter - ?? (!!currentUserId && pendingRequest.submitter_id === currentUserId) - : false; + const isSubmitter = isSubmitterOf(pendingRequest, currentUserId) ?? false; const handleRemind = React.useCallback(async () => { if (!pendingRequest) return; diff --git a/packages/app-shell/src/views/RecordDetailView.approvalRecallGate.test.tsx b/packages/app-shell/src/views/RecordDetailView.approvalRecallGate.test.tsx new file mode 100644 index 0000000000..bc1b496bbc --- /dev/null +++ b/packages/app-shell/src/views/RecordDetailView.approvalRecallGate.test.tsx @@ -0,0 +1,275 @@ +/** + * 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. + */ + +/** + * The record page resolves WHO MAY SEE the recall lever, and threads that + * verdict to the approval band (objectui#6464). + * + * `DetailView.approvalRecallGate` pins what the band does with the verdict, and + * `useRecordApprovals.isSubmitterOf` pins how the verdict is derived. Neither + * can catch a host that derives it correctly and forgets to thread it — the + * whole defect lives in that seam, so this suite mounts the real record page + * against a stubbed approvals API and reads the verdict off the live + * `InlineEditContext`. + * + * The band itself is rendered through the page's schema tree; stubbing + * `SchemaRenderer` with a PROBE instead of `null` keeps the mount cheap while + * still measuring the one value the seam carries. What the probe renders is + * the tri-state verbatim, so `false` (a resolved non-submitter) never reads the + * same as `undefined` (a host that resolved nothing). + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, cleanup } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +const SIGNED_IN_USER = 'u_qcdir'; + +const authFetchSpy = vi.fn(async () => + new Response(JSON.stringify({ data: {} }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), +); +vi.mock('@object-ui/auth', () => ({ + useAuth: () => ({ user: { id: 'u_qcdir', name: 'QC Director', image: null }, activeOrganization: null }), + createAuthenticatedFetch: () => authFetchSpy, +})); + +vi.mock('@object-ui/collaboration', () => ({ + useRecordPresence: () => [], + PresenceAvatars: () => null, +})); + +vi.mock('sonner', () => ({ + toast: Object.assign(vi.fn(), { + success: vi.fn(), error: vi.fn(), info: vi.fn(), + warning: vi.fn(), loading: vi.fn(), dismiss: vi.fn(), + }), +})); + +vi.mock('./ActionConfirmDialog', () => ({ ActionConfirmDialog: () => null })); +vi.mock('./ActionParamDialog', () => ({ ActionParamDialog: () => null })); +vi.mock('./ActionResultDialog', () => ({ ActionResultDialog: () => null })); +vi.mock('./FlowRunner', () => ({ FlowRunner: () => null })); +vi.mock('./MetadataInspector', () => ({ + MetadataPanel: () => null, + useMetadataInspector: () => ({ showDebug: false, toggle: () => {} }), +})); +vi.mock('../hooks/useActionModal', () => ({ + useActionModal: () => ({ + modalHandler: vi.fn(async () => ({ success: true })), + modalElement: null, + closeModal: () => {}, + resolveModalTarget: vi.fn(async () => null), + }), +})); +vi.mock('../utils/consoleServerAction', () => ({ + createConsoleServerActionHandler: () => vi.fn(async () => ({ success: true })), +})); + +/** + * The probe. It stands exactly where the page body (and with it the approval + * band) would render, inside the page's own ``, and prints + * the threaded verdict rather than the band — so this file measures the seam + * and `DetailView.approvalRecallGate` measures the band. + */ +vi.mock('@object-ui/react', async (importOriginal) => { + const actual = await importOriginal(); + const Probe = () => { + const inline = actual.useInlineEdit(); + return ( +
+ ); + }; + return { ...actual, SchemaRenderer: Probe }; +}); + +import { MetadataCtx } from '@object-ui/react'; +import { RecordDetailView } from './RecordDetailView'; + +const OBJECT_NAME = 'qif_report'; +const RECORD_ID = 'QIF202607310002'; +const REQUEST_ID = 'req_qif_1'; + +const OBJECTS = [ + { + name: OBJECT_NAME, + label: 'QIF Report', + fields: { id: { type: 'text', label: 'Id' }, name: { type: 'text', label: 'Name' } }, + }, +]; + +/** + * The pending request as the server sends it. `submitter_id` is deliberately + * NOT the signed-in user in the default row: the discriminating case is a + * reader who is not the submitter, which is the whole field report. + */ +const pendingRequest = (over: Record = {}) => ({ + id: REQUEST_ID, + process_name: 'flow:qif_quality_review', + object_name: OBJECT_NAME, + record_id: RECORD_ID, + status: 'pending', + submitter_id: 'u_inspector', + current_step: 'qc_director_review', + pending_approvers: ['position:qc_director'], + lock_record: true, + ...over, +}); + +let approvalsFetch: ReturnType; + +function stubApprovalsApi(row: Record | null) { + approvalsFetch = vi.fn(async (url: string) => { + const u = String(url); + if (u.includes(`/approvals/requests/${REQUEST_ID}/actions`)) { + return { ok: true, json: async () => ({ data: [] }) } as any; + } + if (u.endsWith(`/approvals/requests/${REQUEST_ID}`)) { + // `getRequest` — the read that attaches the `viewer` block. + return { ok: true, json: async () => row } as any; + } + if (u.includes('/approvals/requests?object=')) { + return { ok: true, json: async () => ({ data: row ? [row] : [] }) } as any; + } + return { ok: true, json: async () => ({ data: [] }) } as any; + }); + vi.stubGlobal('fetch', approvalsFetch); +} + +const METADATA = { + objects: OBJECTS, + pages: [], + loading: false, + error: null, + refresh: async () => {}, + invalidate: () => {}, + ensureType: async () => [], + getItem: async () => null, + getItemsByType: () => [], +} as any; + +function makeDataSource() { + return { + find: vi.fn(async () => ({ data: [] })), + findOne: vi.fn(async () => ({ id: RECORD_ID, name: 'Incoming batch 0731', approval_status: 'pending' })), + create: vi.fn(async () => ({})), + update: vi.fn(async () => ({})), + delete: vi.fn(async () => ({})), + cancelPendingApproval: vi.fn(async () => ({ requestId: REQUEST_ID, status: 'recalled' })), + } as any; +} + +function renderRecordPage() { + return render( + + + {}} + objectNameOverride={OBJECT_NAME} + recordIdOverride={RECORD_ID} + embedded + /> + + , + ); +} + +/** + * Read the verdict off the live context. Waiting on `approval-pending` first is + * what keeps every assertion below a MEASUREMENT: the approvals read is async, + * and a probe queried before it lands reports `undefined` for every case — + * which happens to be the right answer for one of them, so a bare read would + * make the "no approvals API" case pass without ever exercising it. + */ +async function settledProbe(expectPending: boolean) { + const probe = await screen.findByTestId('inline-probe'); + await waitFor(() => + expect(probe.getAttribute('data-approval-pending')).toBe(String(expectPending)), + ); + return probe; +} + +beforeEach(() => { + cleanup(); + authFetchSpy.mockClear(); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +describe('record page → band: who may see recall (objectui#6464)', () => { + /** + * THE DEFECT, at the seam. A reader who is not the submitter: the server + * resolved `is_submitter: false`, and the page must carry that verdict down + * to the band rather than leaving it unset. + */ + it('threads a resolved NON-submitter verdict to the band', async () => { + stubApprovalsApi(pendingRequest({ viewer: { can_act: true, is_submitter: false, can_override: false } })); + renderRecordPage(); + + const probe = await settledProbe(true); + expect(probe.getAttribute('data-approval-is-submitter')).toBe('false'); + }); + + /** The opposite failure: the submitter must not lose the lever. */ + it('threads a resolved SUBMITTER verdict to the band', async () => { + stubApprovalsApi(pendingRequest({ viewer: { can_act: false, is_submitter: true, can_override: false } })); + renderRecordPage(); + + const probe = await settledProbe(true); + expect(probe.getAttribute('data-approval-is-submitter')).toBe('true'); + }); + + /** + * Older server, no `viewer` block: the page falls back to comparing the row's + * `submitter_id` against the signed-in id — and must reach the SAME verdict, + * both ways. + */ + it('falls back to the id comparison when the server sends no viewer block', async () => { + stubApprovalsApi(pendingRequest({ submitter_id: SIGNED_IN_USER })); + renderRecordPage(); + + const probe = await settledProbe(true); + expect(probe.getAttribute('data-approval-is-submitter')).toBe('true'); + }); + + it('resolves a non-submitter through the fallback too', async () => { + stubApprovalsApi(pendingRequest({ submitter_id: 'u_inspector' })); + renderRecordPage(); + + const probe = await settledProbe(true); + expect(probe.getAttribute('data-approval-is-submitter')).toBe('false'); + }); + + /** + * No pending request to consult — the band runs off the record's own + * `approval_status` mirror. The page resolves NO identity here and must + * thread `undefined`, which the band reads as "unchanged from before this + * gate existed". Threading `false` instead would hide recall from the + * submitter on every backend without an approvals API. + */ + it('threads `undefined` — not `false` — when there is no request to consult', async () => { + stubApprovalsApi(null); + renderRecordPage(); + + // Still pending: `approval_status: 'pending'` on the record is the mirror + // that keeps the band up with no approvals row behind it. + const probe = await settledProbe(true); + expect(probe.getAttribute('data-approval-is-submitter')).toBe('undefined'); + }); +}); diff --git a/packages/app-shell/src/views/RecordDetailView.tsx b/packages/app-shell/src/views/RecordDetailView.tsx index ec01558d6c..003daffa14 100644 --- a/packages/app-shell/src/views/RecordDetailView.tsx +++ b/packages/app-shell/src/views/RecordDetailView.tsx @@ -44,7 +44,7 @@ import { AUDIT_FIELD_NAMES, HIDDEN_SYSTEM_FIELD_NAMES } from './record-detail-sy import type { FeedItem } from '@object-ui/types'; import type { ActionDef, ActionParamDef, ConfirmationHandler } from '@object-ui/core'; import type { ConsoleActionDispatch } from '../consoleActionDispatch.js'; -import { useRecordApprovals, recordLockedByApproval } from '../hooks/useRecordApprovals.js'; +import { useRecordApprovals, recordLockedByApproval, isSubmitterOf } from '../hooks/useRecordApprovals.js'; import { RecordAttachmentsPanel } from './RecordAttachmentsPanel.js'; import { RecordApprovalsPanel } from './RecordApprovalsPanel.js'; import { DeclaredActionsBar } from './DeclaredActionsBar.js'; @@ -1023,6 +1023,21 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri // whether their click closes the step. Server-computed; `first_response` // nodes carry none and the band then shows nothing extra. const approvalProgress = approvals.pendingRequest?.decision_progress; + // Who may RECALL the pending approval (objectui#6464). Recall is the + // submitter's lever and the server refuses everyone else, so a non-submitter + // reading a pending record was being offered a button whose click could only + // fail. Same source order the approvals panel's Remind gate uses — one + // `isSubmitterOf` for both, so the two levers cannot disagree about who + // submitted. + // + // `undefined` when there is no pending request to consult: the band is then + // running off the record's `approval_status` mirror alone (a backend with no + // approvals API), the host has resolved no identity, and the DetailView keeps + // its pre-#6464 behaviour rather than hiding on absent information. + // + // This gates the AFFORDANCE only. `canEdit` / `approvalLocked` below are + // untouched by it, and the recall endpoint authorizes the recall itself. + const approvalIsSubmitter = isSubmitterOf(approvals.pendingRequest, user?.id); // A decision landed through the declared-action bar (objectui#3055). The // action itself already POSTed and the runtime already toasted; what the HOST @@ -2322,6 +2337,7 @@ export function RecordDetailView({ dataSource, objects, onEdit, objectNameOverri locked={approvalLocked} approvalPending={approvalPending} approvalProgress={approvalProgress} + approvalIsSubmitter={approvalIsSubmitter} lockedReason={t('detail.lockedTooltip', { defaultValue: 'This record has a pending approval request; editing is locked', })} diff --git a/packages/plugin-detail/README.md b/packages/plugin-detail/README.md index e1276a0cba..8a3b059fe5 100644 --- a/packages/plugin-detail/README.md +++ b/packages/plugin-detail/README.md @@ -294,6 +294,16 @@ shared draft (`InlineEditProvider` from `@object-ui/react`), committed by - **Approval lock**: hosts pass `locked` / `lockedHint` to the save bar and gate `InlineEditProvider.canEdit` when the record is approval-locked, so a locked record hides its edit affordances instead of rejecting at Save. +- **Approval band & recall** (#6464): the band reads `approvalPending` / + `approvalProgress` from the same provider, and offers its **Recall** button + only where the click can succeed. Recall is the submitter's lever — the + server authorizes it on submitter identity and refuses everyone else — so + hosts that resolve approvals pass `InlineEditProvider.approvalIsSubmitter`. + It is tri-state on purpose: `false` withdraws the button from a resolved + non-submitter, `true` keeps it, and **omitting it leaves the button exactly + as it was** — a host that resolves no approval identity is unchanged rather + than losing its submitter's only unlock lever. It gates the affordance only; + the recall endpoint remains the authority on who may actually recall. ## Links diff --git a/packages/plugin-detail/src/DetailView.tsx b/packages/plugin-detail/src/DetailView.tsx index 22003959c5..d10f659449 100644 --- a/packages/plugin-detail/src/DetailView.tsx +++ b/packages/plugin-detail/src/DetailView.tsx @@ -1280,8 +1280,31 @@ export const DetailView: React.FC = ({ {/* Recall belongs to the approval, not to the lock: an editable - pending approval is just as recallable as a locked one. */} - {dataSource?.cancelPendingApproval && ( + pending approval is just as recallable as a locked one. + + It also belongs to the SUBMITTER (objectui#6464). The server + authorizes recall on submitter identity and refuses everyone + else, so rendering the button for every reader of a pending + record offers a lever whose click must fail — the + writability-feedback mismatch, not a permission question: what + the server permits is unchanged by this gate, and nothing + downstream treats `approvalIsSubmitter` as authorization. + + Withdrawn rather than disabled-with-reason, matching the + sibling submitter levers (the approvals panel's Remind, the + declared `approval_recall` action's `visible` predicate): for a + non-submitter this control is never actionable on any pending + record, so a permanently disabled button would be standing + clutter on a band that already states the record is in + approval. The band, its tally and the approvals timeline still + explain the state to everyone; only the lever they can never + pull is gone. + + `undefined` means the host resolves no approval identity, and + then this renders exactly as it did before the signal existed + — hiding on absent information would take the submitter's own + unlock lever away (see InlineEditContextValue). */} + {dataSource?.cancelPendingApproval && inline?.approvalIsSubmitter !== false && (