chore(APICP): revamp the portal UI with a scope-aware app shell - #3281
Conversation
- Restructure the pages, components, hooks and contexts. - Restructure pages, components, hooks, and contexts - Fix issues introduced by the restructuring - Enhance project listing with pagination, sorting, and improved UI - Add an index to the project API resource section - Add form controls to project-related forms - Temporarily make the app sidebar static
…ome restructuring - Make the App Side Bar and Header consistent with the bijira. - Updated projectPath and apiPath functions to manage unresolved project and API scopes. - Refactored routes to utilize new path builders for project and API-level pages. - Improved ConsoleScopeContext and ConsoleScopeProvider to handle new API capabilities and project management. - Added ScopeGate component to manage access to pages based on selected scopes.
|
Important Review skippedToo many files! This PR contains 166 files, which is 66 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (166)
You can disable this status message by setting the Comment |
… resource exporting
AnuGayan
left a comment
There was a problem hiding this comment.
Review — scope-aware app shell
A large app-shell rewrite: three-cluster sidebar with submenus, searchable org/project/API header switchers, an explicit scope model (projectPath/apiPath + ScopeGate + the select-scope marker), and a src/features/* → src/pages/appShell/appShellPages/* restructure.
The scope model is the strongest part. Generating route patterns and sidebar match predicates from one builder (projectScopedPaths/apiScopedPaths/apiScopeSelectPaths) closes a whole class of drift, and the SELECT_SCOPE_SEGMENT rationale in paths.ts — spelling out why dropping segments would make /projects/apis read positionally as project apis — is exactly the reasoning that usually gets lost. toRouteRegex escapes metacharacters before substituting :param, in the right order. ConfirmDialog's rework to a real <form> with useId()-bound labels is a genuine a11y improvement, and ProjectListPage (debounced server-side search, page reset on filter change, page > lastPage clamping after a delete, defineMessages for every string) reads like the standard the other new pages should be held to.
Against that, the branch does not build reproducibly, ships a placeholder page copy-pasted from the AI Workspace portal with an inert primary button, reverts the extension seam #3259 just landed, and deletes ~875 lines of tests for pages it rewrites.
Checks I ran on c8fba03
| Command | Result |
|---|---|
npm ci |
❌ fails — lockfile out of sync with the pinned versions |
npm run typecheck |
✅ clean |
npm test |
✅ 59 files, 732 tests pass (17 MUI out-of-range warnings) |
npm run lint |
❌ 2 errors, both in files added by this PR |
npm run i18n:extract |
en.json 21 lines stale |
Findings
13 inline comments below, each with a reproduction and a paste-ready prompt. Headlines:
- 🔴
npm cifails —c8fba03pinned versions without regeneratingpackage-lock.json.Dockerfile:27andapip-api-control-plane-release.yml:53both runnpm ci, so the image and the release job break. - 🟠 The #3259 Slot/Port seam is reverted, not built on —
render(port)→element,scope→level,useSettingsTabs.tsxand its test deleted,src/slots/orphaned. - 🟠 "Deploy to Gateway" does nothing —
ApiDetailPage.tsx:159has its link commented out. - 🟠 A failed project load shows "Loading…" forever —
AppHeader.tsx:240folds the error into the loading flag.
One finding I could not anchor inline (deleted files)
Severity: 🟠 major
Subject: Six test files deleted with no replacement, and the PR body describes one of them as "expanded"
Description
Deleted with no equivalent anywhere on the branch: ApiCreatePage.test.tsx (100), ApiDetailPage.test.tsx (101), ApiListPage.test.tsx (118), OverviewTab.test.tsx (218), DeployPage.test.tsx (232), AppRoutes.settingsTab.test.tsx (106). The pages themselves survive in rewritten form — ApiListPage.tsx is +86/−112, DeployPage.tsx +37/−11, ApiDetailPage.tsx is brand new — so the rewrite lands untested. The PR body's Tests section lists ApiListPage.test.tsx under "Expanded", but no file by that name exists on the branch.
How to verify
git ls-tree -r --name-only 44a453e -- portals/api-control-plane/src | grep -E '\.test\.tsx?$' \
| sed 's|.*/||' | sort -u > /tmp/base.txt
git ls-tree -r --name-only c8fba03 -- portals/api-control-plane/src | grep -E '\.test\.tsx?$' \
| sed 's|.*/||' | sort -u > /tmp/pr.txt
comm -23 /tmp/base.txt /tmp/pr.txt
find portals/api-control-plane/src -name 'ApiListPage.test.tsx' # no outputSuggested fix
Port the deleted suites to the new file locations. At minimum restore ApiListPage.test.tsx and DeployPage.test.tsx, whose pages changed substantially, and correct the PR body's Tests section.
Prompt for Claude
This branch deletes six test files without replacing them. Restore the two highest-value ones.
1. First confirm they are gone: `find portals/api-control-plane/src -name 'ApiListPage.test.tsx'
-o -name 'DeployPage.test.tsx'`. If either exists, stop and tell me which — the finding is
partly wrong.
2. Recover the originals:
`git show 44a453e:portals/api-control-plane/src/features/apis/ApiListPage.test.tsx`
`git show 44a453e:portals/api-control-plane/src/features/deploy/DeployPage.test.tsx`
3. Port each to its page's new location (src/pages/appShell/appShellPages/apis/ and .../deploy/),
updating import paths and any assertions the rewrite invalidated. Where the page's behaviour
genuinely changed, update the assertion rather than deleting the test.
4. Do not weaken assertions just to make them pass, and do not add `.skip`.
5. Confirm with `npm test` — all suites must pass and the file count must rise from 59.
Nits, not worth their own thread
aiWorkspace.*message ids leak in:ApiDetailPage.tsx:127,138(one namesLLMProxyOverview),ProjectQuickSelector.tsx:126,153,159,167,APIQuickSelector.tsx,ExploreMoreCard.tsx:99. Every other id usesappShell.*/apiControlPlane.*— worth renaming before these reach a translation memory.ExploreMoreCard.tsx:80-90links to AI Workspace docs — "Manage Existing App LLM Proxies" pointing at.../ai-workspace/llm-proxies/manage-proxy/. Wrong product for this page.- Hardcoded English despite the "all shell strings moved to react-intl" claim: every
ScopeGateprompt(12 call sites —promptis typedstring, so<FormattedMessage>can't be passed; widening it toReactNodewould fix that),ScopeGate'sDEFAULT_PROMPT/"Unable to load projects"/"Loading projects", all ofrestApiDisplay.ts('REST API','Published','Not deployed','Unknown'— these render straight into chips),SettingsPage.tsxin full,AppLayout.tsx:164"You're all caught up.",SaveBar.tsx'Save changes'/'Saving…'.ProjectQuickSelector.tsx:134hardcodesplaceholder="Search"where the API one usesintl.formatMessage. src/theme/receipes.tsis misspelled (recipes), andglassSurfaceSxis exported fromtheme/index.tswith no consumer.src/theme/emotionCache.tscarries authoring leftovers: a redundant// src/theme/emotionCache.tspath header, and// ...read the per-request nonce from a meta tag (see §10)— a cross-reference to a document that isn't in the repo. The code also reads.nonceoff ameta[property="csp-nonce"]element; if the nonce is carried incontent=(the usual convention) that yields'', not the nonce. Unverified — I found nocsp-noncemeta inindex.html, so this path may never fire today.AppHeader.tsx:229,308— the clearIconButtoniswidth: 20, height: 10; the 10px height looks like a typo and gives a sub-minimum tap target.ApiDetailPage.tsx:38-48—truncateProviderDisplayNameis declared inside the component body, after the early returns, at the wrong indent level. It's a pure helper that belongs at module scope, and it truncates an API display name, not a provider's.useNavigationItems.ts:139-146— the sort comparator callscombinedRegistry.find()twice per comparison. Fine at ~20 items, but a precomputedMap<id, order>would be faster and clearer.
Reviewed at c8fba03 against base 44a453e, with a local install (npm install), typecheck, full test run, lint and i18n extract. Findings marked "Confirmed" were reproduced; anything I could not reproduce is labelled unverified inline.
- scope: reserve `new` as a non-handle path segment, so the API create page (.../apis/new) no longer reads back as an API called "new", which had put a phantom entry in the header switcher and breadcrumbs, turned isApiScope on mid-creation, and fired a GET for that API - shell: hide breadcrumbs on full-page creation flows (newApi, newGateway) - header: give apiOptions the current-value fallback orgOptions already had, so the API switcher never renders an out-of-range value on first paint - header: fix isProjectsLoading so ProjectQuickSelector's error branch is reachable instead of showing "Loading..." forever on a failed query - apis: wire the inert "Deploy to Gateway" button to routes.apiDeploy - i18n: split scopeGate.noProjects/noApis, each carrying two different sentences - some code refactoring
…and configure a formatter
dee386c to
c2b7c5b
Compare
- Boundary around the routed page in AppLayout, above Suspense, so the shell survives and stale lazy() chunks are caught too. - Recovery actions that work: Try again / Go home, or Reload for a stale chunk. - resetKeys keyed on pathname, so a fallback clears on navigation. - Header switchers and sidebar guarded separately; switchers extracted to HeaderScopeSwitchers so logout stays reachable. Both degrade visibly. - Raw error.message shown in dev builds only.
Purpose
Revamp the API Control Plane portal's app shell so it reads and behaves like Bijira, and restructure the source tree around it. The sidebar, header and layout are the visible half; underneath, the scope model (organization → project → API) is made explicit in the route builders, the navigation registry and a new
ScopeGate, so what the sidebar offers and what a page can actually render can no longer disagree.Resolves https://github.com/wso2-enterprise/apim-saas/issues/2897, https://github.com/wso2-enterprise/apim-saas/issues/2907
App shell
Sidebar
Header
SearchableComplexSelect(icon + primary/secondary rows, bounded trigger width, ellipsized option text) in place of the plainComplexSelect.ProjectQuickSelector,APIQuickSelector) appear for the levels not yet selected, with their own loading/error states.displayNamewith a fallback to the handle, and keep the current value selectable before the list loads so the switcher never renders out of range.react-intl(brand title, switcher labels, search / empty / no-results copy, notification tooltip, footer links, panel title).Layout
PageContent(full width,Stack-spaced above the outlet) instead of floating above it, and now label fromdisplayName.Scope model, routes and gating
projectPath/apiPathbuilders with an explicit unresolved-scope marker. Anullhandle produces the page's scope-less alias by substituting a reservedselect-scopesegment (/organizations/acme/select-scope/apis) rather than dropping segments, which would make/organizations/acme/projects/apisread positionally as projectapis, and would collide project Settings/Home with their organization-level equivalents.ScopeGaterenders a project/API picker in place of the page body until the route carries the scope the page needs, then navigates to the fully-scoped URL.projectScopedPaths/apiScopedPaths/apiScopeSelectPathsgenerate both the route patternsAppRoutesregisters and thematchpredicates the sidebar highlights on, from the same builder, the route table and the highlight can't drift apart. Submenu parents match only the aliases, so parent and child never both claim active.ConsoleScopeProvider/ConsoleScopeContextnow exposeisProjectScope/isApiScope, projects and their error state, and API capabilities, typed against the generated resource clients (RestApi,Organization,Project) rather than hand-written domain types.observe/runtimelogsto API-scopedobservability/logs.ComingSoonplaceholder pages for Admin, API Insights, Compliance, Metrics, Alerts, Monetize, LifeCycle, API Console and API Chat, so every sidebar destination resolves to something.Restructuring
src/features/*→src/pages/appShell/appShellPages/*(apis, deploy, gateways, projects, organizations, observability, insights, manage, test, admin, settings), with each page's owncomponents/andutils/subfolders.src/features/auth→src/contexts/auth;features/billing/ProductActivation→src/hooks; layouts →src/pages/appShell;src/pages/authfor login/callback.src/api/resources/*(organizations, projects, restApis, apiKeys, gateways).Theme
sxrecipes (glassSurfaceSx,hairline,interactiveCardSx,stickyBottomBarSx) so surfaces stay consistent across pages.Documentation
N/A
Tests
AppSidebar.test.tsx,ScopeGate.test.tsx,paths.test.ts,useNavigationItems.test.tsx.navigationRegistry.test.ts(clusters, submenus, adaptive items, alias matching),ApiListPage.test.tsx,ProjectListPage.test.tsx,NewProjectDialog.test.tsx,apiCapabilities.test.ts.Related
main.Test environment