Uh oh!
There was an error while loading. Please reload this page.
fix(app-shell): a denied attachment list is not an empty one - #4685
Merged
Conversation
A `sys_attachment` list read refused for authorization reasons was swallowed
into RecordAttachmentsPanel's empty state: a member denied the parent record
saw "No attachments yet. Upload a file to get started." on a record holding
2095+ attachments, plus an Upload affordance the server would refuse.
The 403 arrives as a throw — the ObjectStack adapter's `find()` degrades only
a non-authz 404 to `{ data: [], total: 0 }` and rethrows the rest — so the
panel's bare `catch { setRows([]) }` was where the two verdicts merged.
Classify it with the house `isPermissionError` predicate (the same one kanban,
calendar and form branch on), render a distinct denied state, and withdraw the
Upload affordance. The denied state shows the i18n sentence and nothing from
the error body — no status code, no server text, no row count. Non-authz
failures keep their pre-existing handling, pinned by a test.
New `detail.attachmentsAccessDenied` key in en + all ten locale packs.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RnQd8iMMUwXQEV1crFmQiQThe latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 15, 2026 08:54
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#4269
A member denied the parent record opened a record holding 2095+ attachments. The
sys_attachmentlist read answered 403, andRecordAttachmentsPanelrendered "No attachments yet. Upload a file to get started." — plus an Upload affordance the server would refuse. An operator could not tell "nothing here" from "not yours".Premises, re-derived on
origin/mainbefore buildingThe card's run was 4 days old, so all three were re-measured against current
main(b8818171f):A — how the 403 reaches the component. It throws.
ObjectStackAdapter.find()degrades only a non-authz 404 to{ data: [], total: 0 }and rethrows everything else (that carve-out is objectui#4408's, so anenable-block denial is not mistaken for a missing collection). So the panel's barecatch { setRows([]) }was exactly where "denied" and "empty" were merged. Confirmed at source, not assumed.B — what gates the Upload affordance today. Nothing. Its only condition was
disabled={uploading}; it rendered unconditionally for every caller. So hiding it under a denied list disturbs no existing permission-based visibility — this is the sole condition added to a control that had none.C — is there a house denied-state pattern? Yes, and it is reused rather than reinvented:
isPermissionErrorfrom@object-ui/react(HTTP 403 /PERMISSION_DENIED/FORBIDDEN/ an RLS denial) is the same predicateObjectKanban,ObjectCalendarand the form renderer branch on, and theLock-icon denial framing mirrorsObjectDataPage's route gate.The change
refresh()classifies a failed list read withisPermissionErrorand sets alistDeniedstate; a successful read clears it.Lockicon, checked before the empty state.detail.attachmentsAccessDeniedkey inen+ all ten locale packs (ar, de, en, es, fr, ja, ko, pt, ru, zh), matching the panel's existinguseObjectTranslationmechanism.No leaking. objectui#2532's failure mode (raw error dump, status code, leaked rows) was absent before and stays absent: the denied state renders the translated sentence and nothing sourced from the error, and
setErroris deliberately not called on this path. Pinned by a test asserting the panel's text contains no403, no2095, no user id and no server message, and that noalertrole appears.Scope. This is the denied-vs-empty distinction only, not a general error-state framework. Non-authz failures (network, 5xx, 401) keep their pre-existing handling, pinned as-is by a test with a comment saying it is a pin rather than an endorsement. That remaining swallow is the same defect class one status over and is filed separately as #4684 — out of scope here, not fixed in this PR.
Verification
All gates run after the final commit, at
6e3013925(working tree clean).vitest run packages/app-shell/.../RecordAttachmentsPanel.test.tsxvitest run packages/i18n/type-check(app-shell + i18n, closure built first)eslint --quiet(all 12 changed source files)pnpm check:i18n-keysenvaluepnpm check:i18n-driftenvalues changedpnpm check:control-bytesnode scripts/check-changeset-presence.mjsDependency closure was built before testing (
pnpm --filter '@object-ui/app-shell^...' build).Reverse verification — direction predicted first
Predicted: reverting only the component and keeping the tests turns exactly the 4 tests that require the denied state red, while the 200-empty test and the non-authz pin stay green (the pin asserts unchanged behavior, so it must pass on both trees).
Observed, exactly: 4 failed | 10 passed. The failure dumps reproduce the reported symptom verbatim — on a 403 the reverted component rendered
Uploadand "No attachments yet. Upload a file to get started." The fix was then restored byte-identically (git diff HEADempty) and re-run green.Generated by Claude Code
Generated by Claude Code