Fix UI issues in Settings, Views and Application pages - #3123
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe change adds a configurable IdP organization identifier, dynamic view fallback and handle renaming, view-scoped API resolution, multi-key-manager UI scoping, sidebar state restoration, documentation, and integration coverage. ChangesView lifecycle and scoped access
IdP organization identity configuration
Multi-key-manager application UI
Sidebar state and navigation styling
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Portal
participant ViewDAO
Client->>Portal: Request organization or view content
Portal->>ViewDAO: Resolve fallback handle or view-scoped artifact
ViewDAO-->>Portal: Handle or visible artifact
Portal-->>Client: Redirect, content, or 404
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
portals/api-portal/it/rest-api/views-and-labels/view-fallback-and-delete.spec.js (1)
19-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftTest fallback and last-view behavior in an isolated organization.
The current redirect assertions run while
defaultexists. The old hardcoded redirect would satisfy them. The suite also omits the final-view deletion guard. Use an isolated organization with nodefaultview, assert the exact fallback handle, and assert that deleting the final view returns 400.Also applies to: 40-67
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@portals/api-portal/it/rest-api/views-and-labels/view-fallback-and-delete.spec.js` around lines 19 - 30, Update the tests in the view fallback and deletion suite to create and use an isolated organization whose views do not include the “default” handle. Assert that the bare organization and portal roots redirect to the exact earliest-created fallback handle, then remove that view and verify deleting the final remaining view returns HTTP 400. Avoid relying on the shared seeded organization or hardcoded default-view redirects.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@portals/api-portal/src/controllers/apiContentController.js`:
- Around line 467-470: In the loadAPIDefinitionRaw function's error handler
(around lines 1562-1570), preserve the original error status code when catching
errors from resolveApiIdInView instead of always sending a 500 response. Check
if the caught error has a specific status code like 404 and propagate that
status in the response rather than overwriting it with 500, so excluded APIs
return the expected 404 instead of 500.
In `@portals/api-portal/src/middlewares/registerPartials.js`:
- Around line 203-206: The registerDocsPageContent middleware currently loads
document content using apiDao.getId without enforcing view boundaries, allowing
excluded APIs to have their content preloaded. Replace the apiDao.getId call
with apiDao.getIdInView and include req.params.viewName as the third parameter
to enforce the view boundary before loading document partials. This ensures the
middleware fails when the API is not present in the requested view instead of
allowing the document content to be loaded.
In `@portals/api-portal/src/services/apiMetadataService.js`:
- Around line 1719-1736: The transaction around the view deletion flow must
serialize the last-view check with concurrent deletions. Update the transaction
handling used by the enclosing delete-view method around viewDao.count and
viewDao.delete to use SERIALIZABLE isolation with appropriate retry handling, or
apply the supported adapter’s organization/views row-lock mechanism; preserve
the existing 404, last-view 400, workflow 409, and successful deletion behavior.
---
Nitpick comments:
In
`@portals/api-portal/it/rest-api/views-and-labels/view-fallback-and-delete.spec.js`:
- Around line 19-30: Update the tests in the view fallback and deletion suite to
create and use an isolated organization whose views do not include the “default”
handle. Assert that the bare organization and portal roots redirect to the exact
earliest-created fallback handle, then remove that view and verify deleting the
final remaining view returns HTTP 400. Avoid relying on the shared seeded
organization or hardcoded default-view redirects.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f55d5e47-7e1b-434a-a0d8-cac559c65f0b
📒 Files selected for processing (44)
portals/api-portal/configs/config-template.tomlportals/api-portal/docs/administer/manage-organizations.mdportals/api-portal/docs/api-portal-openapi-spec-v0.9.yamlportals/api-portal/it/rest-api/organizations/organizations.spec.jsportals/api-portal/it/rest-api/views-and-labels/view-fallback-and-delete.spec.jsportals/api-portal/it/rest-api/views-and-labels/view-handle-resolution.spec.jsportals/api-portal/it/rest-api/views-and-labels/view-scoped-detail-pages.spec.jsportals/api-portal/it/rest-api/views-and-labels/views.spec.jsportals/api-portal/it/ui/cypress/e2e/applications/application-flows.cy.jsportals/api-portal/it/ui/cypress/e2e/applications/key-managers-multiple.cy.jsportals/api-portal/it/ui/cypress/e2e/settings/001-views-labels.cy.jsportals/api-portal/src/app.jsportals/api-portal/src/config/configDefaults.jsportals/api-portal/src/controllers/apiContentController.jsportals/api-portal/src/controllers/apiKeysPageController.jsportals/api-portal/src/controllers/applicationsContentController.jsportals/api-portal/src/controllers/viewConfigureController.jsportals/api-portal/src/dao/apiDao.jsportals/api-portal/src/dao/organizationDao.jsportals/api-portal/src/dao/viewDao.jsportals/api-portal/src/defaultContent/layout/main.hbsportals/api-portal/src/defaultContent/pages/mcp/partials/mcp-listing.hbsportals/api-portal/src/defaultContent/partials/sidebar.hbsportals/api-portal/src/defaultContent/styles/header.cssportals/api-portal/src/defaultContent/styles/side-bar.cssportals/api-portal/src/middlewares/registerPartials.jsportals/api-portal/src/pages/application/partials/keys-instructions.hbsportals/api-portal/src/pages/application/partials/keys-token.hbsportals/api-portal/src/pages/application/partials/manage-keys-km-card.hbsportals/api-portal/src/pages/settings/partials/cfg-modals.hbsportals/api-portal/src/pages/settings/partials/cfg-organization-panel.hbsportals/api-portal/src/pages/settings/partials/cfg-views-panel.hbsportals/api-portal/src/routes/pages/orgContentRoute.jsportals/api-portal/src/scripts/common.jsportals/api-portal/src/scripts/oauth2-key-generation.jsportals/api-portal/src/scripts/settings-organization.jsportals/api-portal/src/scripts/settings-views.jsportals/api-portal/src/services/adminService.jsportals/api-portal/src/services/apiMetadataService.jsportals/api-portal/src/services/seederService.jsportals/api-portal/src/utils/idpOrgIdPolicy.jsportals/api-portal/src/utils/idpOrgIdPolicy.test.jsportals/api-portal/src/utils/orgContext.jsportals/api-portal/src/utils/util.js
Purpose
$subject
Fixes #3118
Fixes #3117
Also fixes a set of issues reported under this:
Approach