Feature ETP-4520: Window access rendering + capability field visibility#67
Open
Gremiger wants to merge 23 commits into
Open
Feature ETP-4520: Window access rendering + capability field visibility#67Gremiger wants to merge 23 commits into
Gremiger wants to merge 23 commits into
Conversation
Contributor
Author
|
VERDICT: APPROVE TEST RESULTS (this repo's portion):
Cross-repo (schema_forge_core): CLI Cross-repo (com.etendoerp.go): full monorepo SCENARIO VERIFICATION (see full report to coordinator for detail/confidence level per item):
No live browser/E2E session or live webhook call was performed (no reachable running deploy in the time available) — all confidence above is code-level/automated-test-level, not a live manual click-through. No Critical/High bugs found. |
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for runtime, per-role window access enforcement and capability-gated field visibility across Schema Forge’s generator pipeline and the app-shell auth runtime.
Changes:
- Extend
AuthProvider/AuthContextwithwindowAccessandcapabilities, populated via an injectedfetchWindowAccesshook onselectRole(), plus fail-closed hooks (useWindowAccess,useHasCapability) and a reusableWindowAccessGuard. - Wire window-access tier checks into generated window pages (route guard for
"none",readOnlyenforcement for"read-only"), keyed bywindow.id. - Add
visibleWhenCapabilityas an opt-in decisions/contract/frontend hint so consumers can omit gated columns/status pills at runtime.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/app-shell-core/src/runtime/AppShellRuntime.jsx | Forwards auth.fetchWindowAccess down to AuthProvider. |
| packages/app-shell-core/src/runtime/tests/AppShellRuntime.test.jsx | Verifies fetchWindowAccess is forwarded and role selection triggers access map population. |
| packages/app-shell-core/src/auth/AuthContext.jsx | Adds windowAccess/capabilities state and fetchWindowAccess integration at role selection time. |
| packages/app-shell-core/src/auth/useWindowAccess.js | Introduces fail-closed useWindowAccess and useHasCapability hooks. |
| packages/app-shell-core/src/auth/WindowAccessGuard.jsx | Adds a generic “no access” guard UI for window routes. |
| packages/app-shell-core/src/auth/index.js | Exposes the new hooks and guard from the auth package entrypoint. |
| packages/app-shell-core/src/auth/tests/AuthContext.test.jsx | Covers fetchWindowAccess invocation, defaults, failure behavior, and clearing behavior. |
| packages/app-shell-core/src/auth/tests/useWindowAccess.test.jsx | Covers fail-closed semantics for access tiers and capability flags. |
| packages/app-shell-core/src/auth/tests/WindowAccessGuard.test.jsx | Verifies denied rendering behavior and allow-through for "full"/"read-only". |
| cli/src/resolve-curated.js | Copies visibleWhenCapability from decisions into curated schema fields. |
| cli/src/generate-contract.js | Passes visibleWhenCapability through to the frontend contract field definition. |
| cli/src/generate-frontend.js | Emits visibleWhenCapability on columns/status pills and adds window-access tier wiring/guarding when window.id exists. |
| cli/test/resolve-curated-visible-when-capability.test.js | Tests curated-schema passthrough for visibleWhenCapability. |
| cli/test/generate-contract-visible-when-capability.test.js | Tests contract passthrough for visibleWhenCapability. |
| cli/test/generate-frontend-visible-when-capability.test.js | Tests frontend emission for columns and status pills. |
| cli/test/generate-frontend-window-access.test.js | Tests page generation for access-tier routing/readonly behavior. |
| cli/test/generate-frontend.test.js | Makes existing import assertions resilient to added useMemo under window-access wiring. |
| docs/decisions-reference.md | Documents runtime window-access gating and capability-gated visibility. |
| docs/neo-headless-extensibility.md | Adds SFWindowAccessMap to the webhook catalog and notes its consumption path. |
Comment on lines
+56
to
+60
| if (typeof fetchWindowAccess !== 'function') return; | ||
| Promise.resolve(fetchWindowAccess(nextSession)) | ||
| .then((result) => { | ||
| if (result?.windowAccess) setWindowAccess(result.windowAccess); | ||
| if (result?.capabilities) setCapabilities(result.capabilities); |
|
|
||
| `window.id` (the AD_Window_ID, already threaded through `resolve-curated.js`/`generate-contract.js` unconditionally — not a decisions.json key, extracted straight from the raw AD Window) now doubles as the lookup key for a **runtime, per-role access tier**. Whenever a contract carries a real `window.id`, `generatePageComponent` emits: | ||
|
|
||
| - `useWindowAccess(windowId)` (from `@etendosoftware/app-shell-core/auth`), resolving `"none" | "read-only" | "full"` from the `windowAccess` map fetched once, at role-selection time, from `GET /webhooks/SFWindowAccessMap`. |
Comment on lines
+20
to
+24
| export function useWindowAccess(windowId) { | ||
| const { windowAccess } = useAuth(); | ||
| if (!windowId) return 'none'; | ||
| return windowAccess?.[windowId] ?? 'none'; | ||
| } |
Comment on lines
+1
to
+6
| import { test, expect } from 'vitest'; | ||
| import { renderToStaticMarkup } from 'react-dom/server'; | ||
| import { render, screen, act } from '@testing-library/react'; | ||
| import { useLocation } from 'react-router-dom'; | ||
| import { AppShellRuntime } from '../AppShellRuntime.jsx'; | ||
| import { useAuth } from '../../auth/index.js'; |
IrinaUrri
approved these changes
Jul 23, 2026
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
docs/neo-headless-extensibility.md:416
- This doc references
modules/com.etendoerp.go/docs/neo-headless.md, but that path does not exist in this repository, so the reference reads like a broken relative link. Consider clarifying that it lives in the Etendo Go repository (or linking to the correct location) so readers don’t chase a non-existent file here.
All webhooks are invoked via HTTP (see `push-to-neo.js` for programmatic usage from Schema Forge). `SFWindowAccessMap` is the one exception consumed directly by the generated frontend (`AuthContext`'s `fetchWindowAccess`) rather than by Schema Forge tooling — see `docs/decisions-reference.md` (window-access gating) and `modules/com.etendoerp.go/docs/neo-headless.md` §8b for the response shape and resolution order.
---
## Related Documentation
- **API Reference:** `modules/com.etendoerp.go/docs/neo-headless.md`
Comment on lines
+68
to
+73
| Promise.resolve() | ||
| .then(() => fetchWindowAccess(nextSession)) | ||
| .then((result) => { | ||
| setWindowAccess(result?.windowAccess ?? {}); | ||
| setCapabilities(result?.capabilities ?? {}); | ||
| }) |
|
|
||
| Both hooks/the guard fail **closed**: an unloaded `windowAccess` map or a missing `windowId` both resolve `"none"`, never `"full"`. Artifacts without a `window.id` (pre-ETP-4520 extractions, hand-built windows with no backing AD Window) silently keep today's behavior — the feature only activates once `window.id` is present. | ||
|
|
||
| This is not a decisions.json opt-in; it applies automatically to every generated window that has an `window.id`. The one companion piece it still needs from whoever wires up the host app: the `GET /webhooks/SFWindowAccessMap` fetcher must be passed to `AuthProvider` as the `fetchWindowAccess` prop, invoked at the same lifecycle moment `selectedRole.orgList` is populated (see `AuthContext.jsx`'s `selectRole()`). |
Comment on lines
1933
to
1941
| const windowConfig = contract?.frontendContract?.window ?? {}; | ||
| // ETP-4520 — AD_Window_ID (threaded through resolve-curated.js/generate-contract.js | ||
| // already, see schema.window.id). Baked as a literal so useWindowAccess() doesn't | ||
| // depend on the host injecting a matching `window.id` prop at runtime. Gates the whole | ||
| // feature: artifacts extracted before window.id was tracked (or hand-built windows with | ||
| // no backing AD_Window) fall back to today's unrestricted behavior rather than emitting | ||
| // a check against an id we don't have. | ||
| const windowAccessId = windowConfig.id ?? null; | ||
| const documentPreview = windowConfig.documentPreview ?? null; |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
docs/decisions-reference.md:79
- Grammar: “has an
window.id” should be “has awindow.id”.
This is not a decisions.json opt-in; it applies automatically to every generated window that has an `window.id`. The one companion piece it still needs from whoever wires up the host app: the `GET /webhooks/SFWindowAccessMap` fetcher must be passed to `AuthProvider` as the `fetchWindowAccess` prop, invoked at the same lifecycle moment `selectedRole.orgList` is populated (see `AuthContext.jsx`'s `selectRole()`).
Comment on lines
39
to
43
| const logout = useCallback(() => { | ||
| const clearedSession = normalizeAuthSession(); | ||
| setSessionState(clearedSession); | ||
| authStorage.clear(); | ||
| onSessionChange?.(clearedSession); |
| | `SFWindowAccessMap` | Per-role window access tier + capability flags (ETP-4520) | -- | | ||
|
|
||
| All webhooks are invoked via HTTP (see `push-to-neo.js` for programmatic usage from Schema Forge). | ||
| All webhooks are invoked via HTTP (see `push-to-neo.js` for programmatic usage from Schema Forge). `SFWindowAccessMap` is the one exception consumed directly by the generated frontend (`AuthContext`'s `fetchWindowAccess`) rather than by Schema Forge tooling — see `docs/decisions-reference.md` (window-access gating) and `modules/com.etendoerp.go/docs/neo-headless.md` §8b for the response shape and resolution order. |
| screen.getByTestId('select-role').click(); | ||
| }); | ||
|
|
||
| expect(await screen.findByTestId('window-access')).toHaveTextContent('{"147":"full"}'); |
Comment on lines
+6
to
+10
| // DATA annotation onto column/statusPill entries here; the actual runtime | ||
| // omission (calling useHasCapability and skipping the render) happens inside | ||
| // DataTable.jsx / DetailView.jsx, which live in the functional repo's | ||
| // tools/app-shell/src/components/contract-ui — NOT in this repo. See the | ||
| // delivery report for the companion change those components still need. |
Comment on lines
+20
to
+24
| // ETP-4520 — request-sequencing guard against a stale-response race: if | ||
| // role A is selected then role B before A's fetchWindowAccess resolves, A's | ||
| // slower response can land AFTER B's and overwrite B's correct maps with | ||
| // A's stale ones. Every selectRole() call increments this ref immediately; | ||
| // only the response whose captured id still matches the ref's CURRENT value |
Gremiger
force-pushed
the
feature/ETP-4520
branch
from
July 23, 2026 17:41
332643a to
893f36c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
packages/app-shell-core/src/runtime/tests/AppShellRuntime.test.jsx:102
- This assertion is likely flaky:
findByTestId('window-access')resolves immediately because the element is rendered from the start, butwindowAccessupdates asynchronously (fire-and-forget fetch). This can assert before the text content changes.
Wait for the updated text instead (e.g., findByText on the expected JSON string, or waitFor around toHaveTextContent).
expect(await screen.findByTestId('window-access')).toHaveTextContent('{"147":"full"}');
docs/decisions-reference.md:79
- Grammar: use “a
window.id” (not “anwindow.id”).
This is not a decisions.json opt-in; it applies automatically to every generated window that has an `window.id`. The one companion piece it still needs from whoever wires up the host app: the `GET /webhooks/SFWindowAccessMap` fetcher must be passed to `AuthProvider` as the `fetchWindowAccess` prop, invoked at the same lifecycle moment `selectedRole.orgList` is populated (see `AuthContext.jsx`'s `selectRole()`).
Comment on lines
+8
to
+10
| // Core vitest runs without `globals: true` (see vitest.config.js) — do | ||
| // explicit cleanup so mounted providers don't bleed between tests. | ||
| afterEach(cleanup); |
Comment on lines
+7
to
+9
| // Core vitest runs without `globals: true` (see vitest.config.js) — do | ||
| // explicit cleanup so mounted providers don't bleed between tests. | ||
| afterEach(cleanup); |
The functional repo's DataTable/DetailView consumption (via the new capabilityVisibility.js helper, commit c6e2ac1e3) closed the gap this note previously flagged as pending; corrects the hook name too.
generatePageComponent tripped Sonar S3776 (18 > 15 allowed) after the windowAccess route-guard/readOnly wiring was added inline. Extracted the four windowAccessId ternaries into buildWindowAccessWiring(), a pure helper with no behavior change.
Clears windowAccess/capabilities before the new role's fetch starts so a role switch never briefly shows the previous role's access map, and defers the fetchWindowAccess() call into the promise chain so a synchronous throw is caught the same way a rejected promise is.
Gremiger
force-pushed
the
feature/ETP-4520
branch
from
July 24, 2026 12:39
893f36c to
4bccf1f
Compare
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
Contributor
|
📦 Preview packages published — dist-tag To exercise the published-package path (no make bump-core-version VERSION=0.3.17-preview.feature-ETP-4520.20260724123958.4bccf1fRe-posted on each push to this branch; supersedes older previews. |
RomanMagnoli
approved these changes
Jul 24, 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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Part A — generic window-level access rendering:
AuthContext/AuthProvidergainwindowAccess/capabilitiesstate, populated via afetchWindowAccesscallback fired fromselectRole()useWindowAccess(windowId)(→ none/read-only/full) anduseHasCapability(key)hooks, both fail-closedWindowAccessGuardcomponent +generate-frontend.jswiring: route guard for"none", forcesreadOnlyfor"read-only"— generic to every generated window with awindow.idPart B — capability-gated field visibility:
decisions.jsonfield propertyvisibleWhenCapability: stringresolve-curated.js→generate-contract.js→generate-frontend.js, onto grid columns and status-pill/extraBadgesentriesVersion: manual minor bump signaled via
packages/schema-forge-core/package.json(lockstep resolver's dedicated signal file) →0.4.0.Part of ETP-4520 (Roles & Users Phase 5) — companion PRs in
com.etendoerp.goandetendo_schema_forge. Full cross-repo design:docs/plans/ETP-4520-cross-domain.mdinetendo_schema_forge.Jira: ETP-4520
Known follow-up (non-blocking)
Reviewed and confirmed:
selectRole()'sfetchWindowAccesscall has no stale-response guard — rapid role-switching before the first fetch resolves could let a slower response overwrite the map for the wrong role. Not currently exploitable (no caller does rapid switching yet), but should be hardened before a role-picker UI ships.Test plan
packages/app-shell-coreauth suite: 24/24 pass (AuthContext, useWindowAccess, useHasCapability, WindowAccessGuard, AppShellRuntime wiring)clisuite: 2523/2523 pass, including 4 new dedicated test files for the generator wiring4c0bca195)