Standardize portal property names and improve code consistency #322
Conversation
WalkthroughBackend: UpdateDevPortal now guards APIKey assignment to avoid persisting masked values. Frontend: portal types moved into feature modules; ApiPublish and DevPortal contexts/hooks reworked (array-shaped publications, new methods), parseApiError added, many UI components updated to use new payloads/fields and isEnabled semantics. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–75 minutes Areas to focus during review:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
DakshithaS
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portals/management-portal/src/pages/portals/PortalForm.tsx (1)
129-138: Error message doesn't account for edit mode.The fallback error message is "Failed to create Developer Portal." regardless of whether the form is in create or edit mode. This could confuse users editing a portal.
const errorMessage = error instanceof Error ? error.message - : 'Failed to create Developer Portal.'; + : isEdit + ? 'Failed to update Developer Portal.' + : 'Failed to create Developer Portal.'; showNotification(errorMessage, 'error');portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (1)
34-47: Unused prop:relativeTimeis defined but never used.The
relativeTimefunction is declared in Props and destructured but is never called in the component. Either remove it from the props or utilize it (e.g., for displaying relative timestamps forpublication.publishedAt).type Props = { portal: ApiPublicationWithPortal; apiId: string; publishingIds: Set<string>; - relativeTime: (d?: Date) => string; onPublish: (portal: ApiPublicationWithPortal) => void; }; const DevPortalDeployCard: React.FC<Props> = ({ portal, apiId, publishingIds, - onPublish, + onPublish, }) => {Or if it should be used, apply it to display relative time for publications.
🧹 Nitpick comments (13)
platform-api/src/internal/service/devportal_service.go (1)
344-346: API key masking guard works but clarifying “clear” semantics and edge cases would helpThe new condition correctly prevents a masked placeholder (all
*) from overwriting the stored API key. A couple of behavioral nuances are worth calling out:
- This also prevents clearing the API key via an empty string: if the frontend ever tries to “remove” the key by sending
"", the update will be ignored and the existing key retained. If the product should support explicitly clearing the key (similar to how some fields are intentionally clearable via empty strings), this path will need a separate flag or different condition. Based on learnings, description updates are explicitly allowed to clear via empty strings, so the asymmetry here should be deliberate.- Inputs like
"******** "or" "(trailing/only spaces) will pass the current check and overwrite the real key with a non-usable value. If the UI might ever add whitespace, you may want to normalize withTrimSpacebefore theTrim('*')check, or centralize this logic in a small helper so frontend/backend stay consistent on what counts as a “masked” value.If the intent is “only update when a real, non-empty, non-placeholder key is provided and never allow clearing”, then this is fine as-is; otherwise, consider tightening the predicate or adding explicit “clear key” semantics.
portals/management-portal/src/pages/portals/PortalCard.tsx (1)
69-70: EmptytestIdprops should be meaningful or removed.Passing empty strings to
testIdprovides no testing value. Either assign meaningful identifiers for test automation or remove them:- <Card testId={''} style={{ maxWidth: 450 }}> - <CardActionArea onClick={onClick} testId={''}> + <Card testId="portal-card" style={{ maxWidth: 450 }}> + <CardActionArea onClick={onClick} testId="portal-card-action">portals/management-portal/src/pages/portals/PortalList.tsx (2)
20-28: Unusederrorprop in interface.The
errorprop is defined inPortalListProps(line 23) but is never destructured or used in the component. Consider removing it if error handling is managed elsewhere, or implement the error display if it's intended functionality.interface PortalListProps { portals: Portal[]; loading: boolean; - error?: string | null; onPortalClick: (portalId: string) => void; onPortalActivate: (portalId: string) => void; onPortalEdit: (portalId: string) => void; onCreateNew: () => void; }
192-203: Redundant dialog close calls.
confirmationDialog.onConfirm()(line 196) already closes the dialog via line 92, butcloseConfirmationDialog()(line 197) is called immediately after. While this is harmless, it's redundant. Consider either removing the second close call or restructuring so the dialog'sonConfirmonly runs the action without closing.<ConfirmationDialog open={confirmationDialog.open} onClose={closeConfirmationDialog} - onConfirm={() => { - confirmationDialog.onConfirm(); - closeConfirmationDialog(); - }} + onConfirm={confirmationDialog.onConfirm} title={confirmationDialog.title} message={confirmationDialog.message} confirmText={confirmationDialog.confirmText} severity={confirmationDialog.severity} />portals/management-portal/src/hooks/apiPublish.ts (1)
68-76: Weak typing onlistproperty.Using
any[]for thelistproperty defeats the purpose of TypeScript's type safety. Consider using a more specific type.export type PublicationsListResponse = { count: number; - list: any[]; + list: ApiPublicationWithPortal[]; pagination?: { total: number; offset: number; limit: number; } | null; };portals/management-portal/src/pages/apis/ApiPublish/DevPortalPickTable.tsx (1)
61-65: Comment-code mismatch: "active" vs "isEnabled".The comment on line 61 says "active portals" but the code now filters by
isEnabled. Update the comment to maintain consistency with the property name standardization.- // Only show NOT published AND active portals + // Only show NOT published AND enabled portals const visiblePortals = React.useMemo( () => portals.filter((p) => !publishedSet.has(p.uuid) && p.isEnabled), [portals, publishedSet] );portals/management-portal/src/pages/apis/ApiPublish.tsx (1)
256-288: Consider extracting portal-to-publication mapping to a reusable utility.The inline construction of
ApiPublicationWithPortalfromPortalis verbose. A mapper function (possibly inApiPublish/mapper.ts) would improve maintainability and ensure consistency if this transformation is needed elsewhere.Example extraction:
// In mapper.ts export const portalToUnpublishedPublication = (portal: Portal): ApiPublicationWithPortal => ({ uuid: portal.uuid, name: portal.name, identifier: portal.identifier, description: portal.description, portalUrl: portal.uiUrl || '', apiUrl: portal.apiUrl || '', hostname: portal.hostname || '', isEnabled: portal.isEnabled || false, createdAt: portal.createdAt || '', updatedAt: portal.updatedAt || '', associatedAt: '', isPublished: false, publication: { status: 'UNPUBLISHED', apiVersion: '', sandboxEndpoint: '', productionEndpoint: '', publishedAt: '', updatedAt: '', }, });portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (3)
53-67: Consider using constants for status values.The status comparison uses lowercase strings (
'published','failed') from the API while displaying uppercase ('PUBLISHED','FAILED'). Consider using constants fromPORTAL_CONSTANTSfor consistency and maintainability.
109-116: Remove commented-out code.The commented-out
logoSrcconditional is leftover development code. Remove it for cleaner code.- {/* {logoSrc ? ( */} <Box component="img" src={BijiraDPLogo} alt="Bijira Dev Portal Logo" sx={{ width: 90, height: 90, objectFit: 'contain' }} /> - {/* ) : null} */}
264-264: Use single quotes for consistency.The button text uses double quotes, inconsistent with the PR's single-quote standardization.
- {isPublishingThis ? "Adding…" : "Add API"} + {isPublishingThis ? 'Adding…' : 'Add API'}portals/management-portal/src/hooks/devportals.ts (1)
1-1: File header comment mismatch.The comment says
useDevPortalsApi.tsbut the file isdevportals.ts. Update for accuracy.-// 📁 useDevPortalsApi.ts +// 📁 devportals.tsportals/management-portal/src/context/ApiPublishContext.tsx (2)
101-106: Consider logging refresh failures.While silently ignoring refresh errors after a successful publish is acceptable for UX, consider logging the error for debugging purposes.
try { await refreshPublishedApis(apiId); } catch { - /* ignore refresh error */ + // Publish succeeded but refresh failed - user may need to manually refresh + console.warn('Failed to refresh publications after publish'); }
198-203: Use single quotes for consistency.The error message uses double quotes, inconsistent with the PR's single-quote standardization.
export const useApiPublishing = () => { const ctx = useContext(ApiPublishContext); if (!ctx) { - throw new Error("useApiPublishing must be used within an ApiPublishProvider"); + throw new Error('useApiPublishing must be used within an ApiPublishProvider'); } return ctx; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
platform-api/src/internal/service/devportal_service.go(1 hunks)portals/management-portal/src/components/ErrorBoundary.tsx(2 hunks)portals/management-portal/src/constants/portal.ts(2 hunks)portals/management-portal/src/context/ApiPublishContext.tsx(2 hunks)portals/management-portal/src/context/DevPortalContext.tsx(2 hunks)portals/management-portal/src/context/NotificationContext.tsx(4 hunks)portals/management-portal/src/hooks/apiPublish.ts(3 hunks)portals/management-portal/src/hooks/devportals.ts(1 hunks)portals/management-portal/src/pages/PortalManagement.tsx(6 hunks)portals/management-portal/src/pages/apis/ApiPublish.tsx(10 hunks)portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx(4 hunks)portals/management-portal/src/pages/apis/ApiPublish/DevPortalPickTable.tsx(13 hunks)portals/management-portal/src/pages/apis/ApiPublish/mapper.ts(3 hunks)portals/management-portal/src/pages/overview/utils.ts(1 hunks)portals/management-portal/src/pages/overview/widgets/APIPortalWidget.tsx(8 hunks)portals/management-portal/src/pages/portals/PortalCard.tsx(9 hunks)portals/management-portal/src/pages/portals/PortalForm.tsx(7 hunks)portals/management-portal/src/pages/portals/PortalList.tsx(6 hunks)portals/management-portal/src/pages/portals/PortalPreviews.tsx(6 hunks)portals/management-portal/src/pages/portals/PromoBanner.tsx(4 hunks)portals/management-portal/src/pages/portals/ThemeContainer.tsx(4 hunks)portals/management-portal/src/pages/portals/ThemeSettingsPanel.tsx(4 hunks)portals/management-portal/src/types/portal.ts(0 hunks)portals/management-portal/src/utils/apiErrorUtils.ts(1 hunks)portals/management-portal/src/utils/portalUtils.ts(2 hunks)
💤 Files with no reviewable changes (1)
- portals/management-portal/src/types/portal.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-05T05:26:25.128Z
Learnt from: thivindu
Repo: wso2/api-platform PR: 101
File: platform-api/src/internal/handler/project.go:169-169
Timestamp: 2025-11-05T05:26:25.128Z
Learning: In platform-api/src/internal/handler/project.go UpdateProject handler, the UpdateProjectRequest allows empty description field by design. This is intentional to provide a means to clear/empty the description of a project. The frontend is responsible for populating the existing description value when the user is not modifying it, implementing a full-replacement update pattern rather than partial/PATCH-style updates.
Applied to files:
platform-api/src/internal/service/devportal_service.go
📚 Learning: 2025-12-02T07:06:41.830Z
Learnt from: piyumaldk
Repo: wso2/api-platform PR: 223
File: portals/management-portal/src/common/VersionInput.tsx:23-31
Timestamp: 2025-12-02T07:06:41.830Z
Learning: In portals/management-portal/src/common/VersionInput.tsx, the touchedRef pattern is intentionally used for one-time initialization. Once the component loads, only user modifications should update the value, and external prop changes are deliberately ignored to prevent disrupting user input.
Applied to files:
portals/management-portal/src/components/ErrorBoundary.tsxportals/management-portal/src/utils/portalUtils.tsportals/management-portal/src/pages/portals/PortalPreviews.tsxportals/management-portal/src/pages/portals/PortalForm.tsxportals/management-portal/src/pages/portals/ThemeSettingsPanel.tsxportals/management-portal/src/pages/portals/PromoBanner.tsxportals/management-portal/src/pages/portals/PortalCard.tsxportals/management-portal/src/context/DevPortalContext.tsxportals/management-portal/src/pages/apis/ApiPublish/DevPortalPickTable.tsxportals/management-portal/src/pages/portals/PortalList.tsxportals/management-portal/src/pages/overview/widgets/APIPortalWidget.tsxportals/management-portal/src/pages/portals/ThemeContainer.tsxportals/management-portal/src/pages/PortalManagement.tsx
📚 Learning: 2025-11-06T07:04:01.101Z
Learnt from: DakshithaS
Repo: wso2/api-platform PR: 105
File: platform-api/src/internal/model/api_publication.go:63-79
Timestamp: 2025-11-06T07:04:01.101Z
Learning: In the API Platform codebase (platform-api/src/internal/model/api_publication.go), the PublicationStateMachine intentionally disallows Published → Publishing transitions. The design requires APIs to be explicitly unpublished before they can be republished, enforcing the lifecycle: Published → Unpublishing → Unpublished → Publishing → Published. This prevents accidental overwrites and ensures proper cleanup.
Applied to files:
portals/management-portal/src/hooks/apiPublish.tsportals/management-portal/src/pages/apis/ApiPublish.tsx
🧬 Code graph analysis (10)
portals/management-portal/src/utils/apiErrorUtils.ts (1)
platform-api/src/internal/dto/openapi_spec.go (1)
Response(105-108)
portals/management-portal/src/pages/apis/ApiPublish/mapper.ts (1)
portals/management-portal/src/hooks/apiPublish.ts (1)
ApiPublishPayload(46-66)
portals/management-portal/src/pages/portals/PortalForm.tsx (4)
portals/management-portal/src/hooks/devportals.ts (2)
CreatePortalPayload(47-56)UpdatePortalPayload(59-59)portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-53)portals/management-portal/src/context/NotificationContext.tsx (1)
useNotifications(68-76)portals/management-portal/src/utils/portalUtils.ts (1)
slugify(110-117)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (1)
portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-53)
portals/management-portal/src/pages/portals/PortalCard.tsx (1)
portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-53)
portals/management-portal/src/context/DevPortalContext.tsx (2)
portals/management-portal/src/hooks/devportals.ts (4)
Portal(10-27)CreatePortalPayload(47-56)UpdatePortalPayload(59-59)useDevPortalsApi(82-213)portals/management-portal/src/context/NotificationContext.tsx (1)
useNotifications(68-76)
portals/management-portal/src/hooks/apiPublish.ts (1)
portals/management-portal/src/utils/apiErrorUtils.ts (1)
parseApiError(5-34)
portals/management-portal/src/pages/overview/widgets/APIPortalWidget.tsx (1)
portals/management-portal/src/context/DevPortalContext.tsx (1)
useDevPortals(225-231)
portals/management-portal/src/hooks/devportals.ts (1)
portals/management-portal/src/utils/apiErrorUtils.ts (1)
parseApiError(5-34)
portals/management-portal/src/pages/PortalManagement.tsx (4)
portals/management-portal/src/context/DevPortalContext.tsx (1)
useDevPortals(225-231)portals/management-portal/src/utils/portalUtils.ts (4)
getPortalMode(67-87)getPortalIdFromPath(92-105)navigateToPortalTheme(43-50)navigateToPortalEdit(55-62)portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-53)portals/management-portal/src/hooks/devportals.ts (2)
CreatePortalPayload(47-56)UpdatePortalPayload(59-59)
🔇 Additional comments (36)
portals/management-portal/src/pages/portals/PromoBanner.tsx (1)
1-67: Quote standardization applied cleanly—LGTM.All changes are cosmetic quote conversions (double → single) consistent with the PR's formatting standardization goals. No functional, behavioral, or type-safety modifications; component structure and logic remain intact.
portals/management-portal/src/utils/portalUtils.ts (1)
21-27: ✓ Formatting refactoring is clean and consistent.The multi-line parameter formatting across navigation functions and the conditional refactoring in
getPortalIdFromPathimprove readability and align with the PR's standardization goals. All type annotations are maintained, and the utility patterns remain effective—particularly the reuse ofgetPortalListPathto avoid duplication.Also applies to: 32-38, 43-50, 55-62, 98-103
portals/management-portal/src/pages/portals/ThemeSettingsPanel.tsx (1)
1-59: LGTM! Formatting standardization applied consistently.All changes in this file are cosmetic, converting double-quoted strings to single quotes across import statements, string literals, and MUI selectors. This aligns with the PR's objective of improving formatting consistency across the codebase.
portals/management-portal/src/pages/portals/ThemeContainer.tsx (2)
10-14: LGTM! Type localization improves modularity.Moving
ThemeContainerPropsfrom the external types module to a local declaration aligns well with the PR objectives to decentralize types. The interface signature remains unchanged, preserving the component's public API while improving code organization.
31-32: LGTM! Formatting consistency improved.The multi-line error message formatting enhances readability while preserving the existing error handling logic.
Also applies to: 43-44
portals/management-portal/src/constants/portal.ts (1)
56-59: LGTM! Type helper formatting improvements.The parentheses around the indexed access types improve readability and follow TypeScript conventions for complex type expressions.
portals/management-portal/src/context/NotificationContext.tsx (1)
19-65: LGTM! Formatting standardization with no behavioral changes.The component logic remains sound with proper
useCallbackmemoization and context error handling.portals/management-portal/src/components/ErrorBoundary.tsx (1)
1-73: LGTM! Formatting-only changes.Error boundary implementation remains correct with proper lifecycle methods for error capture and state reset.
portals/management-portal/src/pages/portals/PortalPreviews.tsx (1)
1-119: LGTM! Formatting standardization applied consistently.Quote style and spacing adjustments maintain the existing preview component behavior.
portals/management-portal/src/pages/portals/PortalCard.tsx (2)
23-38: Well-structured interface with clear prop naming.The
PortalCardPropsinterface properly documents the component's API, and theenablednaming aligns with the PR's standardization goals.
158-213: URL handling logic correctly gates access based onenabledstate.The conditional rendering for tooltips, link behavior, and icon button states properly prevents interaction when the portal is not enabled.
portals/management-portal/src/pages/overview/utils.ts (1)
16-17: Good defensive improvement with optional parameter.Returning an empty string for undefined input is a safe fallback. Ensure callers expecting a non-empty time string handle this case appropriately in the UI.
portals/management-portal/src/pages/overview/widgets/APIPortalWidget.tsx (1)
119-123: Good security practice withnoopener,noreferrer.The
window.opencall correctly usesnoopener,noreferrerto prevent the opened page from accessing the opener window, mitigating potential tabnabbing attacks.portals/management-portal/src/pages/portals/PortalList.tsx (1)
72-102: Activation flow handles async correctly with proper cleanup.The
requestActivatecallback properly uses a try/finally block to ensureactivatingstate is cleaned up regardless of success or failure, and the dialog is closed after activation completes.portals/management-portal/src/hooks/apiPublish.ts (2)
104-110: Consistent error handling withparseApiError.Good standardization—all API methods now use
parseApiErrorfor consistent error message extraction, improving the user experience when API calls fail.
147-161: Verify ifdevPortalIdis required in both URL path and request body.The
devPortalIdis included in the URL path (line 152) and duplicated in the request body (line 159). This pattern is unusual—typically path parameters aren't repeated in the body. Confirm whether the API actually requires this duplication, as it may indicate either intentional defensive design or an error where the body should contain different data.portals/management-portal/src/pages/portals/PortalForm.tsx (2)
85-93: URL validation is thorough and well-implemented.The validation correctly:
- Ensures the URL is well-formed via
new URL()- Restricts protocols to
http:orhttps:- Rejects trailing slashes
This prevents common URL format issues.
112-117: Good practice: Stripping masked API key from update payload.Correctly removes the masked placeholder from the payload before submission, ensuring the backend won't receive or process the mask as an actual key value.
portals/management-portal/src/pages/PortalManagement.tsx (1)
180-193: Critical: API key mask mismatch between components.The masked
apiKeyis set to'**********'(10 asterisks) here, butPortalForm.tsx(lines 84 and 114) checks for'*****'(5 asterisks). This mismatch will cause:
- The form validation to incorrectly require a new API key in edit mode
- The API key to be included in update requests when it shouldn't be
Either update this mask to match PortalForm's expectation:
initialData={ selectedPortal ? { name: selectedPortal.name, identifier: selectedPortal.identifier, description: selectedPortal.description, apiUrl: selectedPortal.apiUrl, hostname: selectedPortal.hostname, - apiKey: '**********', // Masked for security + apiKey: '*****', // Masked for security headerKeyName: selectedPortal.headerKeyName, visibility: selectedPortal.visibility, } : undefined }Or define a shared constant (recommended):
// In constants/portal.ts export const MASKED_API_KEY = '*****';⛔ Skipped due to learnings
Learnt from: piyumaldk Repo: wso2/api-platform PR: 223 File: portals/management-portal/src/common/VersionInput.tsx:23-31 Timestamp: 2025-12-02T07:06:41.830Z Learning: In portals/management-portal/src/common/VersionInput.tsx, the touchedRef pattern is intentionally used for one-time initialization. Once the component loads, only user modifications should update the value, and external prop changes are deliberately ignored to prevent disrupting user input.portals/management-portal/src/pages/apis/ApiPublish/DevPortalPickTable.tsx (2)
18-31: LGTM on Props interface expansion.The Props interface cleanly documents optional props and API compatibility notes. The component correctly computes its own
allSelected/someSelectedinternally based on visible portals, which is the right approach since the parent may not know which portals are filtered out.
81-92: LGTM on select-all logic.The
handleToggleAllVisiblecorrectly scopes selection toggling to only visible (unpublished + enabled) portals, preventing unintended selection of filtered-out items.portals/management-portal/src/pages/apis/ApiPublish.tsx (3)
104-113: LGTM on clear-before-refresh pattern.Calling
clearPublishedApis()beforerefreshPublishedApis(effectiveApiId)ensures a clean state, preventing stale data from a previous API from mixing with the newly fetched publications.
432-438: LGTM on provider composition.The component correctly wraps
DevelopContentwith bothDevPortalProviderandApiPublishProvider, ensuring the necessary context is available.
123-130: Ensure callback dependencies are memoized withuseCallback.The effect depends on multiple callback functions (
fetchApiById,selectApi,refreshDevPortals,refreshPublishedApis,clearPublishedApis). These should be wrapped inuseCallbackin their respective hooks or contexts to prevent unnecessary effect re-runs and maintain referential stability across renders.portals/management-portal/src/hooks/devportals.ts (3)
10-27: LGTM on Portal interface standardization.The
Portalinterface correctly usesisEnabledinstead ofisActive, aligning with the PR's property standardization goal. All relevant fields are properly typed.
47-59: LGTM on payload types.
CreatePortalPayloaddefines all required fields for portal creation, andUpdatePortalPayloadcorrectly usesPartial<CreatePortalPayload>to allow partial updates. TheapiKeyfield in updates enables key rotation.
91-94: LGTM on centralized error handling.Consistent use of
parseApiErroracross all API operations ensures uniform error messages. Including the operation context (e.g.,'fetch devportal (${uuid})') aids debugging.portals/management-portal/src/context/ApiPublishContext.tsx (2)
26-43: LGTM on context type definition.The
ApiPublishContextValuetype is well-structured with clear function signatures. The shift from a per-API map to a flatApiPublicationWithPortal[]simplifies the interface whileclearPublishedApisenables clean state resets.
150-162: LGTM on organization switch handling.Using
prevOrgRefto track the previous organization prevents unnecessary state clears during transient loading states. The effect correctly clears data only when the organization actually changes.portals/management-portal/src/context/DevPortalContext.tsx (7)
1-19: LGTM!Clean import organization. The addition of
useReffor the fetch optimization pattern and the integration withuseOrganization/useNotificationscontexts aligns well with the centralized error handling approach.
25-38: LGTM!The interface cleanly exposes uuid-based methods with proper payload types. The removal of the
errorfield in favor of centralized notification handling is a good separation of concerns.
48-73: LGTM!Good pattern: renaming the API methods (e.g.,
createRequest,updateRequest) avoids naming collisions with the context's public methods. ThehandleErrorcallback properly extracts error messages and delegates to the notification system.
77-90: LGTM!The
refreshDevPortalsimplementation correctly handles the loading state and returns an empty array on error, which is a sensible default for list data. The error is surfaced via notification.
135-152: Verify intended behavior: new portals are prepended to the list.When
fetchDevPortalByIdretrieves a portal not already in state, it prepends to the beginning of the list (line 143). If ordering matters (e.g., sorted bycreatedAt), this could lead to inconsistent display until the next full refresh.If insertion order is intentional for visibility, this is fine. Otherwise, consider triggering a full refresh or inserting at the correct position.
191-218: LGTM!Proper use of
useMemoto stabilize the context value. All dependencies are correctly specified, preventing unnecessary re-renders of consumers.
225-231: LGTM!Standard and correct pattern for context consumption. The descriptive error message aids debugging when the hook is used outside the provider.
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx
Show resolved
Hide resolved
portals/management-portal/src/pages/overview/widgets/APIPortalWidget.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
portals/management-portal/src/pages/portals/PortalForm.tsx (1)
85-93: Consider validating URL format before trailing slash check.The validation logic on line 93 checks
!formData.apiUrl.trim().endsWith('/')before callingisValidUrl. If the URL is invalid (e.g., not a URL at all), the trailing slash check will still pass, potentially causing confusion.Consider reordering the checks:
- const apiUrlValid = formData.apiUrl.trim() && !formData.apiUrl.trim().endsWith('/') && isValidUrl(formData.apiUrl.trim()); + const trimmedUrl = formData.apiUrl.trim(); + const apiUrlValid = trimmedUrl && isValidUrl(trimmedUrl) && !trimmedUrl.endsWith('/');This ensures URL format is validated first, then checks for trailing slash on known-valid URLs. It also avoids multiple
.trim()calls.portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (1)
34-40: UnusedrelativeTimeprop in type definition.The
relativeTimeprop is declared in thePropstype but is never destructured or used in the component body. Either remove it from the type or destructure and use it where needed.type Props = { portal: ApiPublicationWithPortal; apiId: string; publishingIds: Set<string>; - relativeTime: (d?: Date) => string; onPublish: (portal: ApiPublicationWithPortal) => void; };portals/management-portal/src/pages/PortalManagement.tsx (1)
108-131: Consider type refinement forhandleCreatePortal.The function accepts
CreatePortalPayload | UpdatePortalPayloadbut immediately casts toCreatePortalPayload(line 110). Since this handler is only used for creation (line 169), consider accepting onlyCreatePortalPayloaddirectly to avoid the cast.const handleCreatePortal = React.useCallback( - async (formData: CreatePortalPayload | UpdatePortalPayload) => { - const fullData = formData as CreatePortalPayload; + async (formData: CreatePortalPayload) => { setCreatingPortal(true); try { - const createdPortal = await createDevPortal(fullData); + const createdPortal = await createDevPortal(formData);portals/management-portal/src/context/DevPortalContext.tsx (1)
136-153: Review the update-or-prepend logic infetchDevPortalById.The logic at lines 141-144 checks if a portal with
portal.uuidexists, but then updates by matchingp.uuid === uuid(the parameter). Ifportal.uuiddiffers from theuuidparameter (unlikely but possible if API returns different data), this could cause unexpected behavior. Consider using consistent identifiers:setDevportals((prev) => { - const existing = prev.find((p) => p.uuid === portal.uuid); + const existingIndex = prev.findIndex((p) => p.uuid === portal.uuid); return existing - ? prev.map((p) => (p.uuid === uuid ? { ...p, ...portal } : p)) + return existingIndex >= 0 + ? prev.map((p) => (p.uuid === portal.uuid ? { ...p, ...portal } : p)) : [portal, ...prev]; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
portals/management-portal/src/constants/portal.ts(3 hunks)portals/management-portal/src/context/ApiPublishContext.tsx(2 hunks)portals/management-portal/src/context/DevPortalContext.tsx(2 hunks)portals/management-portal/src/hooks/apiPublish.ts(3 hunks)portals/management-portal/src/hooks/devportals.ts(1 hunks)portals/management-portal/src/pages/PortalManagement.tsx(6 hunks)portals/management-portal/src/pages/apis/ApiPublish/ApiPublishModal.tsx(3 hunks)portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx(5 hunks)portals/management-portal/src/pages/apis/ApiPublish/DevPortalPickTable.tsx(13 hunks)portals/management-portal/src/pages/overview/widgets/APIPortalWidget.tsx(8 hunks)portals/management-portal/src/pages/portals/PortalCard.tsx(9 hunks)portals/management-portal/src/pages/portals/PortalForm.tsx(7 hunks)portals/management-portal/src/pages/portals/PortalList.tsx(7 hunks)portals/management-portal/src/pages/portals/ThemeContainer.tsx(4 hunks)portals/management-portal/src/utils/apiErrorUtils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- portals/management-portal/src/pages/portals/ThemeContainer.tsx
- portals/management-portal/src/pages/overview/widgets/APIPortalWidget.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-02T07:06:41.830Z
Learnt from: piyumaldk
Repo: wso2/api-platform PR: 223
File: portals/management-portal/src/common/VersionInput.tsx:23-31
Timestamp: 2025-12-02T07:06:41.830Z
Learning: In portals/management-portal/src/common/VersionInput.tsx, the touchedRef pattern is intentionally used for one-time initialization. Once the component loads, only user modifications should update the value, and external prop changes are deliberately ignored to prevent disrupting user input.
Applied to files:
portals/management-portal/src/pages/apis/ApiPublish/DevPortalPickTable.tsxportals/management-portal/src/pages/portals/PortalCard.tsxportals/management-portal/src/context/DevPortalContext.tsxportals/management-portal/src/pages/PortalManagement.tsxportals/management-portal/src/pages/portals/PortalList.tsxportals/management-portal/src/pages/portals/PortalForm.tsx
📚 Learning: 2025-11-06T07:04:01.101Z
Learnt from: DakshithaS
Repo: wso2/api-platform PR: 105
File: platform-api/src/internal/model/api_publication.go:63-79
Timestamp: 2025-11-06T07:04:01.101Z
Learning: In the API Platform codebase (platform-api/src/internal/model/api_publication.go), the PublicationStateMachine intentionally disallows Published → Publishing transitions. The design requires APIs to be explicitly unpublished before they can be republished, enforcing the lifecycle: Published → Unpublishing → Unpublished → Publishing → Published. This prevents accidental overwrites and ensures proper cleanup.
Applied to files:
portals/management-portal/src/hooks/apiPublish.ts
🧬 Code graph analysis (6)
portals/management-portal/src/pages/apis/ApiPublish/ApiPublishModal.tsx (1)
portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-68)
portals/management-portal/src/pages/portals/PortalCard.tsx (1)
portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-68)
portals/management-portal/src/hooks/apiPublish.ts (1)
portals/management-portal/src/utils/apiErrorUtils.ts (1)
parseApiError(5-34)
portals/management-portal/src/context/ApiPublishContext.tsx (1)
portals/management-portal/src/hooks/apiPublish.ts (5)
ApiPublicationWithPortal(30-44)ApiPublishPayload(46-66)PublishResponse(9-13)UnpublishResponse(15-19)useApiPublishApi(91-178)
portals/management-portal/src/hooks/devportals.ts (1)
portals/management-portal/src/utils/apiErrorUtils.ts (1)
parseApiError(5-34)
portals/management-portal/src/pages/PortalManagement.tsx (4)
portals/management-portal/src/context/DevPortalContext.tsx (1)
useDevPortals(229-235)portals/management-portal/src/utils/portalUtils.ts (4)
getPortalMode(67-87)getPortalIdFromPath(92-105)navigateToPortalTheme(43-50)navigateToPortalEdit(55-62)portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-68)portals/management-portal/src/hooks/devportals.ts (2)
CreatePortalPayload(46-55)UpdatePortalPayload(58-58)
🔇 Additional comments (28)
portals/management-portal/src/pages/apis/ApiPublish/ApiPublishModal.tsx (1)
13-13: LGTM! Centralized messaging improves consistency.The replacement of hard-coded error messages with centralized constants from
PORTAL_CONSTANTS.MESSAGESis a good improvement for maintainability and consistency across the codebase.Also applies to: 115-115, 119-119, 129-129
portals/management-portal/src/utils/apiErrorUtils.ts (1)
5-34: LGTM! Response body handling is correct.The implementation correctly reads the response body as text once (line 11), then attempts JSON parsing on that text. This avoids the stream-consumed-twice issue and follows best practices. The fallback chain for error message extraction is well-designed.
portals/management-portal/src/pages/portals/PortalCard.tsx (1)
23-38: LGTM! Consistent property renaming.The renaming from
isActivetoenabledis applied consistently throughout the component. All conditional logic (URL navigation, tooltips, button states, styling) correctly references the new property. Accessibility features are properly maintained.Also applies to: 56-56, 160-210, 259-260, 265-308
portals/management-portal/src/constants/portal.ts (1)
10-10: LGTM! Centralized constants improve maintainability.The additions to
PORTAL_CONSTANTSprovide good centralization of default values and messages. TheAPI_KEY_MASKconstant (10 asterisks) should ensure consistency across the codebase when checking for masked API keys.Also applies to: 40-57, 71-74
portals/management-portal/src/pages/portals/PortalForm.tsx (2)
84-84: LGTM! API key masking is correctly implemented.The code now uses
PORTAL_CONSTANTS.API_KEY_MASKconsistently in both validation (line 84) and payload sanitization (lines 114-116). This prevents the masked placeholder from being sent to the backend during edits, addressing the concern from previous reviews.Also applies to: 114-116
265-282: LGTM! Visibility field properly integrated.The visibility field UI is well-implemented with proper accessibility (label), state management, and disabled state handling during submission.
portals/management-portal/src/pages/apis/ApiPublish/DevPortalPickTable.tsx (1)
61-65: LGTM! Filter logic correctly updated for enabled portals.The filter on line 63 now correctly checks
p.isEnabledinstead of the deprecatedisActive, ensuring only enabled and unpublished portals are shown. The no-data message accurately reflects this filtering logic.Also applies to: 194-194
portals/management-portal/src/hooks/apiPublish.ts (2)
3-3: LGTM! Consistent error handling with parseApiError.The integration of
parseApiErroracross all three API methods (fetchPublications,publishApiToDevPortal,unpublishApiFromDevPortal) provides consistent error message extraction and handling. This aligns well with the centralized error utility pattern.Also applies to: 104-110, 137-140, 163-166
38-38: Property rename consistently applied.The rename from
isActivetoisEnabledin theApiPublicationWithPortaltype is consistent with the broader refactoring across the PR.portals/management-portal/src/pages/portals/PortalList.tsx (2)
20-27: LGTM! Clean interface definition and property usage.The
PortalListPropsinterface clearly defines the component's public API with well-separated concerns. The use ofportal.isEnabled(line 123) is consistent with the property rename across the codebase.Also applies to: 123-123
71-101: LGTM! Activation flow properly manages async state.The activation flow correctly:
- Sets activating state before the async operation (line 80)
- Cleans up activating state in the finally block (lines 85-89)
- Closes the confirmation dialog after activation (line 91)
- Handles the async operation without blocking the UI
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (3)
76-116: LGTM on the logo block restructuring.The grid-based layout with the logo container is well-structured with proper alignment, borders, and overflow handling. The nested Box components provide good visual hierarchy.
169-186: Good use of centralized constants for accessibility.The use of
PORTAL_CONSTANTS.MESSAGES.OPEN_PORTAL_URLfor the tooltip andPORTAL_CONSTANTS.ARIA_LABELS.OPEN_PORTAL_URLfor the aria-label aligns well with the PR's goal of standardizing portal constants. Thenoopener,noreferrersecurity attributes onwindow.openare correctly applied.
196-242: Status box styling is now correct.The previous dead code branches checking
!isPublishedhave been removed as per the past review. The styling logic now correctly handles only the reachable states within theisPublishedconditional.portals/management-portal/src/hooks/devportals.ts (5)
9-26: Good type definition withisEnabledstandardization.The Portal interface properly uses
isEnabledinstead ofisActive, aligning with the PR's objective to standardize portal property names. The interface is comprehensive with all necessary fields.
28-44: Consider aligning optional fields between Portal and PortalUIModel.
Portal.logoSrcandPortal.logoAltare required (lines 17-18), whilePortalUIModel.logoSrcandPortalUIModel.logoAltare optional (lines 36-37). Ensure this asymmetry is intentional—if API responses can omit logos,Portalshould also mark them optional.
81-97: LGTM on standardized error handling.The use of
parseApiErrorprovides consistent error message extraction across all API operations. The fallback to an empty array whendata.listis undefined is a safe default.
169-182: LGTM on delete operation.The delete operation properly uses
parseApiErrorfor error handling and doesn't expect a response body, which is appropriate for DELETE requests.
184-202: LGTM on activate operation.The activate operation is correctly implemented as a POST with proper error handling and no expected response body.
portals/management-portal/src/context/ApiPublishContext.tsx (4)
26-43: Good API surface cleanup.The context value type is cleaner with
publishedApisas a flat array instead of a Record, and the addition ofgetPublicationandclearPublishedApisprovides better control over the state.
72-73: Good use of ref for organization tracking.Using
prevOrgRefto track organization changes prevents unnecessary state clears on re-renders when the organization hasn't actually changed.
100-107: Appropriate handling of refresh failure after successful publish.The nested try-catch correctly handles the case where publish succeeds but the subsequent refresh fails. Logging a warning is reasonable here since the primary operation succeeded.
151-163: LGTM on organization switch handling.The effect correctly guards against clearing data when the organization hasn't changed by comparing
currentOrgIdwithprevOrgIdstored in the ref. This prevents unnecessary state resets on re-renders.portals/management-portal/src/pages/PortalManagement.tsx (2)
133-150: LGTM on update handler with correct error message.The
handleUpdatePortalnow correctly usesPORTAL_CONSTANTS.MESSAGES.UPDATE_FAILEDfor the fallback error message, addressing the previous review comment about using the wrong constant.
180-193: Good security practice with API key masking.Using
PORTAL_CONSTANTS.API_KEY_MASKfor theapiKeyfield in edit mode prevents exposing the actual API key in the form. This aligns with the PR's goal of standardizing portal constants.portals/management-portal/src/context/DevPortalContext.tsx (3)
68-74: Good centralized error handling pattern.The
handleErrorhelper provides consistent error message extraction and notification display across all operations, reducing code duplication.
155-170: Good use ofisEnabledfor activation.The activation logic correctly updates the portal's
isEnabledfield totrue, aligning with the PR's objective of standardizing the property name fromisActivetoisEnabled.
184-191: LGTM on organization change handling.The ref is now set only after
refreshDevPortals()completes successfully via.then(), addressing the previous review comment about failed fetches preventing automatic retries. The.catch()handler safely ignores errors that are already captured in state.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
portals/management-portal/src/pages/portals/PortalForm.tsx (2)
130-139: Error message fallback doesn't account for edit mode.The fallback error message is hardcoded to "Failed to create Developer Portal" but this handler runs for both create and edit operations. When an update fails, users will see an incorrect message.
const errorMessage = error instanceof Error ? error.message - : 'Failed to create Developer Portal.'; + : isEdit + ? 'Failed to update Developer Portal.' + : 'Failed to create Developer Portal.'; showNotification(errorMessage, 'error');
164-174: Duplicate title displayed in both header and form container.The title "Edit Portal Details" / "Add Your Portal Details" appears twice—once in the header section (line 165) and again in the form container (line 173). This creates visual redundancy. Consider removing one instance.
{/* Form Container */} <Box sx={{ width: 560, maxWidth: '100%', position: 'relative' }}> - {/* Form Title */} - <Typography variant="h5" sx={{ mb: 1 }}> - {isEdit ? 'Edit Portal Details' : 'Add Your Portal Details'} - </Typography> - {/* Form Fields */} <Grid container spacing={2}>
🧹 Nitpick comments (4)
portals/management-portal/src/pages/userScenarios/PublishPortalFlow.tsx (1)
110-132: isEnabled-based filtering for portals is correct and consistentDeriving
portalsfromallPortals.filter(p => p.isEnabled)cleanly scopes the publish flow to enabled portals and aligns with the PR-wideisActive→isEnabledrename. Downstream logic that computesallPublishedToActivePortalsand drives the “No Active Developer Portals” state correctly operates on this filtered list.If you want to polish naming later, you could optionally rename
allPublishedToActivePortalsto reflect “enabled” portals for internal consistency, but that’s non-blocking.portals/management-portal/src/pages/portals/PortalForm.tsx (2)
83-101: URL validation is thorough; consider explicit boolean conversion for consistency.The validation logic correctly handles the masked API key in edit mode and properly validates URL format including protocol and trailing slash. Minor nit:
apiKeyValidevaluates totrueor a string—explicit!!formData.apiKey.trim()would make the intent clearer, though it works correctly with the final!!()wrapper.- const apiKeyValid = isEdit && formData.apiKey === PORTAL_CONSTANTS.API_KEY_MASK ? true : formData.apiKey.trim(); + const apiKeyValid = isEdit && formData.apiKey === PORTAL_CONSTANTS.API_KEY_MASK ? true : !!formData.apiKey.trim();
186-189: Consider adding meaningful test IDs for form inputs.Multiple
TextInputcomponents havetestId="". Meaningful test IDs (e.g.,testId="portal-name-input") would improve component testability and enable more robust integration tests.portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (1)
125-141: Optional: Consider standard approaches for multi-line truncation.The current implementation uses vendor-prefixed WebKit properties (
-webkit-box,WebkitLineClamp,WebkitBoxOrient) for two-line text truncation. While this works in most modern browsers, it relies on vendor-specific CSS.Consider one of the following alternatives if broader compatibility or future-proofing is desired:
- Use the standard
line-clampproperty (check browser support requirements for your project).- Use a utility library like
react-clamp-linesfor more robust cross-browser truncation.- Keep the current approach if it meets your browser support matrix.
The current implementation is acceptable for most use cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
portals/management-portal/src/pages/apis/ApiPublish/ApiPublishModal.tsx(4 hunks)portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx(4 hunks)portals/management-portal/src/pages/portals/PortalForm.tsx(7 hunks)portals/management-portal/src/pages/userScenarios/PublishPortalFlow.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- portals/management-portal/src/pages/apis/ApiPublish/ApiPublishModal.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-02T07:06:41.830Z
Learnt from: piyumaldk
Repo: wso2/api-platform PR: 223
File: portals/management-portal/src/common/VersionInput.tsx:23-31
Timestamp: 2025-12-02T07:06:41.830Z
Learning: In portals/management-portal/src/common/VersionInput.tsx, the touchedRef pattern is intentionally used for one-time initialization. Once the component loads, only user modifications should update the value, and external prop changes are deliberately ignored to prevent disrupting user input.
Applied to files:
portals/management-portal/src/pages/userScenarios/PublishPortalFlow.tsxportals/management-portal/src/pages/portals/PortalForm.tsx
🧬 Code graph analysis (1)
portals/management-portal/src/pages/portals/PortalForm.tsx (3)
portals/management-portal/src/hooks/devportals.ts (2)
CreatePortalPayload(46-55)UpdatePortalPayload(58-58)portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-68)portals/management-portal/src/utils/portalUtils.ts (1)
slugify(110-117)
🔇 Additional comments (6)
portals/management-portal/src/pages/portals/PortalForm.tsx (5)
20-26: Well-defined props interface with appropriate typing.The
PortalFormPropsinterface cleanly separates create vs edit concerns withisEditand properly typesinitialDataasPartial<CreatePortalPayload>to handle partial data in edit mode.
28-58: State initialization and synchronization look correct.The lazy initializer pattern for
useStateand theuseEffectfor syncinginitialDatachanges are properly implemented. Default visibility of'private'aligns withPORTAL_CONSTANTS.DEFAULT_VISIBILITY_LABEL.
60-77: Auto-generation logic correctly scoped to add mode only.The conditional auto-generation of
identifierfromnameusingslugifyis properly gated by!isEdit, ensuring identifiers remain stable during edits.
266-283: Visibility field properly implemented with type-safe value handling.The
SimpleSelectfor visibility correctly uses the type assertion for the constrained union type. The field integrates well with the form state management.
302-318: Submit button provides clear feedback for both modes.The button correctly handles disabled state, loading indicators, and contextual labels for create vs edit operations.
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (1)
70-70: Verify thatapiUrlis a valid fallback for a user-accessible portal URL.The code falls back to
portal.apiUrlwhenportal.portalUrlis unavailable. Confirm thatapiUrlrepresents a user-facing portal URL and not an API endpoint. IfapiUrlis an API endpoint URL, this fallback could result in displaying an incorrect or non-functional link to users.
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx
Show resolved
Hide resolved
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (2)
146-189: Consider adding URL validation before opening portal.The IconButton opens the portal URL in a new tab with proper security flags (
noopener,noreferrer) and accessibility labels. However, there's no validation thatportalUrlis defined or valid before callingwindow.open().Apply this diff to add a defensive check:
<IconButton size="small" sx={{ ml: 0.5 }} onClick={(e: React.MouseEvent) => { e.stopPropagation(); - window.open(portalUrl, '_blank', 'noopener,noreferrer'); + if (portalUrl) { + window.open(portalUrl, '_blank', 'noopener,noreferrer'); + } }} aria-label={PORTAL_CONSTANTS.ARIA_LABELS.OPEN_PORTAL_URL} + disabled={!portalUrl} > <LaunchOutlinedIcon fontSize="inherit" /> </IconButton>This prevents attempting to open an undefined URL and provides visual feedback when the URL is unavailable.
198-244: Consider distinct styling for PENDING and UNKNOWN states.The styling logic treats
UNKNOWNandPENDINGstates identically toFAILED(red error colors), which may confuse users:
- PENDING: Indicates publication is in progress or awaiting status—not necessarily an error. Users might expect neutral (gray) or info (blue) styling.
- UNKNOWN: Indicates an unexpected status value—might warrant warning (yellow/orange) styling to signal something unexpected without implying outright failure.
- FAILED: Correctly uses error (red) styling.
Apply this diff to provide distinct visual treatment:
sx={{ backgroundColor: (t) => status === 'FAILED' ? t.palette.mode === 'dark' ? 'rgba(239,68,68,0.12)' : '#FDECEC' : success ? t.palette.mode === 'dark' ? 'rgba(16,185,129,0.12)' : '#E8F7EC' + : status === 'PENDING' + ? t.palette.mode === 'dark' + ? 'rgba(59,130,246,0.12)' + : '#EFF6FF' : t.palette.mode === 'dark' - ? 'rgba(239,68,68,0.12)' - : '#FDECEC', + ? 'rgba(251,191,36,0.12)' + : '#FFFBEB', border: (t) => `1px solid ${ status === 'FAILED' ? t.palette.error.light : success ? '#D8EEDC' + : status === 'PENDING' + ? t.palette.info.light : t.palette.error.light + : t.palette.warning.light }`,And update the Chip color logic:
<Chip label={status} color={ status === 'FAILED' ? 'error' : success ? 'success' + : status === 'PENDING' + ? 'info' : 'error' + : 'warning' } variant={success ? 'filled' : 'outlined'} size="small" />This provides clearer visual feedback:
- Green: Published (success)
- Red: Failed (error)
- Blue: Pending (in progress)
- Yellow/Orange: Unknown (unexpected)
portals/management-portal/src/pages/portals/PortalForm.tsx (3)
60-77:updateFieldgeneric and auto-slug logic are correct, but may override custom identifiersThe generic
updateFieldoverkeyof CreatePortalPayloadis nicely typed, and restricting auto-slug ofidentifierto add mode (!isEdit) is sensible. One functional nuance: whenever the name changes in add mode, any manual edits toidentifierwill be overwritten byslugify(name). If you expect users to be able to customize the identifier independently after first generation, consider tracking whether the identifier was user-touched and only auto-update while it’s still in its initial state.
83-101: Validation logic is sound; minor boolean clarity improvement possibleThe
apiKeyValidhandling correctly treats the mask value as “still valid” in edit mode and otherwise enforces a non-empty trimmed key, which matches the API key masking semantics. URL validation vianew URLwith protocol checks and a trailing-slash guard is also reasonable. For readability, you might want to coerceapiUrlValidto a pure boolean (e.g.,const apiUrlValid = !!trimmedUrl && isValidUrl(trimmedUrl) && !trimmedUrl.endsWith('/')) so its type doesn’t becomestring | boolean, though the final!!(...)already ensuresisValidis strictly boolean.
266-283: Visibility select wiring is correct; consider using the payload type for the castThe visibility field is hooked up correctly with
SimpleSelect, and the two options (public/private) align with the initial default. To keep types in sync withCreatePortalPayload, you could cast using the type instead of the literal union:- <SimpleSelect - value={formData.visibility} - onChange={(event) => updateField('visibility', event.target.value as 'public' | 'private')} + <SimpleSelect + value={formData.visibility} + onChange={(event) => + updateField('visibility', event.target.value as CreatePortalPayload['visibility']) + }This way any future change to the visibility union is picked up automatically here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx(4 hunks)portals/management-portal/src/pages/portals/PortalForm.tsx(12 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-12-02T07:06:41.830Z
Learnt from: piyumaldk
Repo: wso2/api-platform PR: 223
File: portals/management-portal/src/common/VersionInput.tsx:23-31
Timestamp: 2025-12-02T07:06:41.830Z
Learning: In portals/management-portal/src/common/VersionInput.tsx, the touchedRef pattern is intentionally used for one-time initialization. Once the component loads, only user modifications should update the value, and external prop changes are deliberately ignored to prevent disrupting user input.
Applied to files:
portals/management-portal/src/pages/portals/PortalForm.tsx
📚 Learning: 2025-12-05T00:18:04.859Z
Learnt from: DakshithaS
Repo: wso2/api-platform PR: 322
File: portals/management-portal/src/pages/apis/ApiPublish/mapper.ts:33-33
Timestamp: 2025-12-05T00:18:04.859Z
Learning: In portals/management-portal/src/pages/apis/ApiPublish/mapper.ts, the labels field is intentionally hardcoded to ['default'] because the dynamic labels feature is not yet implemented, even though FormLike.labels?: string[] exists in the interface.
Applied to files:
portals/management-portal/src/pages/portals/PortalForm.tsx
📚 Learning: 2025-11-06T07:04:01.101Z
Learnt from: DakshithaS
Repo: wso2/api-platform PR: 105
File: platform-api/src/internal/model/api_publication.go:63-79
Timestamp: 2025-11-06T07:04:01.101Z
Learning: In the API Platform codebase (platform-api/src/internal/model/api_publication.go), the PublicationStateMachine intentionally disallows Published → Publishing transitions. The design requires APIs to be explicitly unpublished before they can be republished, enforcing the lifecycle: Published → Unpublishing → Unpublished → Publishing → Published. This prevents accidental overwrites and ensures proper cleanup.
Applied to files:
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx
🧬 Code graph analysis (1)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (1)
portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-68)
🔇 Additional comments (7)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (5)
1-9: LGTM! Imports are consistent and complete.All imports use single quotes as per the standardization effort, and the new imports (IconButton, Tooltip, LaunchOutlinedIcon, PORTAL_CONSTANTS) align with the refactored UI requirements.
11-32: LGTM! Card layout improvements.The explicit flexbox properties ensure consistent card height and pin the action button to the bottom, improving visual consistency across portal cards.
34-46: LGTM! Props type is clean.The removal of the
relativeTimeprop simplifies the component interface. The remaining props are appropriate for the card's functionality.
47-74: LGTM! Status derivation logic properly handles edge cases.The updated logic explicitly handles
UNKNOWNandPENDINGstates instead of defaulting toPUBLISHED, which addresses the previous review concern. This improves reliability by distinguishing between:
- Successful publication (
'published'→PUBLISHED)- Failed publication (
'failed'→FAILED)- Unknown status values (
UNKNOWN)- Missing status (
PENDING)The fallback to
PENDINGwhenpublication.statusis falsy is reasonable for publications that were initiated but haven't reported status yet.
246-256: LGTM! Publish button logic is correct.The button text uses single quotes for consistency, and the disabled state properly prevents publishing when
apiIdis missing or when the operation is already in progress.portals/management-portal/src/pages/portals/PortalForm.tsx (2)
20-26: Props and initial state wiring look solidUsing
Partial<CreatePortalPayload>forinitialDataand seedingformDatafrominitialFormDataplus overrides is clean and type-safe. The addedvisibility: 'private'default also keeps the payload well-defined without depending on callers.Also applies to: 28-37, 46-49, 51-53
188-189: Test IDs, field states, button labels, and export all look consistentAdding stable
testIdprops across inputs, disabling the identifier only in edit mode, and introducing clear button labels (Saving.../Create and continue/Save Changes) make the form easier to test and more understandable for users. The default export ofPortalFormis also straightforward and consistent with the rest of the file.Also applies to: 195-203, 210-218, 224-232, 239-248, 254-262, 285-296, 313-317, 324-324
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (3)
52-69: Enhanced status handling looks good; consider adding debug logging.The status derivation now correctly handles PUBLISHED, FAILED, UNKNOWN, and PENDING states. This addresses the past review feedback about explicit handling of unexpected statuses.
Optional enhancement: Add debug logging when UNKNOWN or PENDING states are encountered to aid troubleshooting:
if (isPublished) { if (publication?.status === 'published') { status = 'PUBLISHED'; success = true; } else if (publication?.status === 'failed') { status = 'FAILED'; success = false; } else if (publication?.status) { status = 'UNKNOWN'; success = false; + console.warn('[DevPortalDeployCard] Unexpected publication status:', publication.status); } else { status = 'PENDING'; success = false; + console.warn('[DevPortalDeployCard] Publication status missing for published portal:', portal.uuid); } }
87-118: Optional: Simplify redundant Box nesting.The logo block uses two nested Boxes with overlapping properties. Consider consolidating:
- <Box - sx={{ - width: 100, - height: 100, - borderRadius: 2, - bgcolor: 'transparent', - display: 'flex', - alignItems: 'center', - justifyContent: 'center', - }} - > <Box sx={{ width: 100, height: 100, borderRadius: '8px', border: '0.5px solid #abb8c2ff', bgcolor: '#d9e0e4ff', overflow: 'hidden', display: 'flex', alignItems: 'center', justifyContent: 'center', }} > <Box component="img" src={BijiraDPLogo} alt="Bijira Dev Portal Logo" sx={{ width: 90, height: 90, objectFit: 'contain' }} /> </Box> - </Box>
201-255: Status box logic is correct but consider extracting for readability.The backgroundColor and border styling correctly handles all four states (FAILED, PUBLISHED, PENDING, UNKNOWN) with appropriate theme-aware colors. However, the deeply nested ternary (16 lines) reduces maintainability.
Optional enhancement: Extract styling logic into a helper function:
const getStatusBoxStyles = (status: string, success: boolean, theme: any) => { const styles = { FAILED: { bg: theme.palette.mode === 'dark' ? 'rgba(239,68,68,0.12)' : '#FDECEC', border: theme.palette.error.light, }, PUBLISHED: { bg: theme.palette.mode === 'dark' ? 'rgba(16,185,129,0.12)' : '#E8F7EC', border: '#D8EEDC', }, PENDING: { bg: theme.palette.mode === 'dark' ? 'rgba(59,130,246,0.12)' : '#EFF6FF', border: theme.palette.info.light, }, UNKNOWN: { bg: theme.palette.mode === 'dark' ? 'rgba(251,191,36,0.12)' : '#FFFBEB', border: theme.palette.warning.light, }, }; return status === 'FAILED' ? styles.FAILED : success ? styles.PUBLISHED : status === 'PENDING' ? styles.PENDING : styles.UNKNOWN; };Then use:
const statusStyles = getStatusBoxStyles(status, success, t); // ... backgroundColor: statusStyles.bg, border: `1px solid ${statusStyles.border}`,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx(4 hunks)portals/management-portal/src/pages/portals/PortalForm.tsx(12 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-06T07:04:01.101Z
Learnt from: DakshithaS
Repo: wso2/api-platform PR: 105
File: platform-api/src/internal/model/api_publication.go:63-79
Timestamp: 2025-11-06T07:04:01.101Z
Learning: In the API Platform codebase (platform-api/src/internal/model/api_publication.go), the PublicationStateMachine intentionally disallows Published → Publishing transitions. The design requires APIs to be explicitly unpublished before they can be republished, enforcing the lifecycle: Published → Unpublishing → Unpublished → Publishing → Published. This prevents accidental overwrites and ensures proper cleanup.
Applied to files:
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx
📚 Learning: 2025-12-02T07:06:41.830Z
Learnt from: piyumaldk
Repo: wso2/api-platform PR: 223
File: portals/management-portal/src/common/VersionInput.tsx:23-31
Timestamp: 2025-12-02T07:06:41.830Z
Learning: In portals/management-portal/src/common/VersionInput.tsx, the touchedRef pattern is intentionally used for one-time initialization. Once the component loads, only user modifications should update the value, and external prop changes are deliberately ignored to prevent disrupting user input.
Applied to files:
portals/management-portal/src/pages/portals/PortalForm.tsx
📚 Learning: 2025-12-05T00:18:04.859Z
Learnt from: DakshithaS
Repo: wso2/api-platform PR: 322
File: portals/management-portal/src/pages/apis/ApiPublish/mapper.ts:33-33
Timestamp: 2025-12-05T00:18:04.859Z
Learning: In portals/management-portal/src/pages/apis/ApiPublish/mapper.ts, the labels field is intentionally hardcoded to ['default'] because the dynamic labels feature is not yet implemented, even though FormLike.labels?: string[] exists in the interface.
Applied to files:
portals/management-portal/src/pages/portals/PortalForm.tsx
🧬 Code graph analysis (1)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (1)
portals/management-portal/src/constants/portal.ts (1)
PORTAL_CONSTANTS(2-68)
🔇 Additional comments (7)
portals/management-portal/src/pages/apis/ApiPublish/DevPortalDeployCard.tsx (2)
1-32: LGTM! Clean import organization and consistent card layout.The Card component's fixed height and flexbox layout ensure visual consistency across multiple portal cards.
146-192: LGTM! Proper URL handling with good UX and security.The portal URL display includes:
- Proper overflow handling with ellipsis
- Accessible Tooltip and IconButton with ARIA labels
- Secure
window.openwithnoopener,noreferrer- Correct disabled state handling
portals/management-portal/src/pages/portals/PortalForm.tsx (5)
17-37: Props and initial payload defaults align with create/edit portal flowsTyping
onSubmitas(CreatePortalPayload | UpdatePortalPayload)withinitialData?: Partial<CreatePortalPayload>and aCreatePortalPayload-shapedinitialFormData(includingvisibility: 'private') is consistent and gives a sensible default state for both add and edit modes. No changes needed here.
46-55: Form state and identifier auto-generation are clean and type-safeUsing
useState<CreatePortalPayload>seeded frominitialFormData/initialDataplus a genericupdateFieldoverkeyof CreatePortalPayloadkeeps the form strongly typed. The identifier auto-generation only whenfield === 'name'in add mode andidentifierTouchedis false gives a predictable UX while still allowing manual overrides. This looks solid.Also applies to: 62-84
90-108: Validation correctly handles API key masking and URL format
apiKeyValidnow correctly treats the masked value viaPORTAL_CONSTANTS.API_KEY_MASKas valid in edit mode while still requiring a non-empty key otherwise, and theapiUrlValidcheck enforces a trimmed HTTP(S) URL without a trailing slash. Combined with non-empty name and hostname checks,isValidis a sensible gate for enabling submission.
112-163: Submit handler cleanly separates create vs edit flows and masks API key updatesBranching on
isEditto construct anUpdatePortalPayload(withapiKeydropped when it equalsPORTAL_CONSTANTS.API_KEY_MASK) versus passingformDatadirectly for creates keeps the payload semantics clear and avoids persisting the mask. Using mode-specific fallback error messages and tying localsubmitStatustoeffectiveSubmittingalso looks correct and addresses earlier concerns about misleading error copy.
191-216: UI additions (test IDs, visibility select, button copy) are coherent and support testingThe added
testIdprops on key inputs, disabling the Identifier field in edit mode, introducing the VisibilitySimpleSelectwithpublic/privateoptions, and adjusting button labels/states (Saving…/Creating…,Save ChangesvsCreate and continue) all align well with the form’s behaviour and should make both UX and automated testing smoother. No issues spotted.Also applies to: 220-247, 249-275, 279-299, 300-314, 323-332
1 participant
Purpose
Refactor management portal codebase to standardize portal property names from
isActivetoisEnabled, improve code consistency with single quotes, enhance error handling with centralized API error parsing, and update type definitions for better maintainability.Goals
Approach
isEnabledinstead ofisActiveapiErrorUtils.tsutility for consistent API error parsingdevportals.ts,apiPublish.ts) with improved error handling usingparseApiErrorPortalCard.tsx,PortalList.tsx,PortalManagement.tsx, etc.) to use new property namestypes/portal.tsfile and moved interfaces to appropriate hook filesUser stories
Documentation
N/A - Internal refactoring with no user-facing changes
Automation tests
Security checks
Samples
N/A - Internal refactoring
Related PRs
N/A
Test environment
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.