Skip to content

Introduce debug feature for FIdP#10277

Open
LinukaAr wants to merge 27 commits into
wso2:feature-connection-debugfrom
LinukaAr:idp-validator
Open

Introduce debug feature for FIdP#10277
LinukaAr wants to merge 27 commits into
wso2:feature-connection-debugfrom
LinukaAr:idp-validator

Conversation

@LinukaAr
Copy link
Copy Markdown
Contributor

Purpose

Adds a debug flow for federated IdPs (FIdPs), enabling administrators to initiate and inspect authentication sessions for a given connection. This helps diagnose misconfigurations and protocol-level issues (e.g. OIDC claim mapping, token exchange failures).

Related Issues

Related PRs

Checklist

  • e2e cypress tests locally verified. (for internal contributers)
  • Manual test round performed and verified.
  • UX/UI review done on the final implementation.
  • Documentation provided. (Add links if there are any)
  • Relevant backend changes deployed and verified
  • Unit tests provided. (Add links if there are any)
  • Integration tests provided. (Add links if there are any)

Security checks

Developer Checklist (Mandatory)

  • Complete the Developer Checklist in the related product-is issue to track any behavioral change or migration impact.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds an end-to-end Identity Provider connection testing feature: debug endpoints and API client, a "Test Connection" action in the connection edit page, a Connection Test results page with caching/polling and three result tabs, styled UI components, JSP popup pages, and routing/config updates.

Changes

Identity Provider Connection Testing

Layer / File(s) Summary
Endpoint and route definitions
features/admin.connections.v1/models/endpoints.ts, features/admin.connections.v1/configs/endpoints.ts, features/admin.core.v1/constants/app-constants.ts, apps/console/src/configs/routes.tsx
Adds debug and debugResult endpoint fields and concrete URLs, registers IDP_TEST path, and wires a lazy-loaded console route for the connection test page.
Connection test API client
features/admin.connections.v1/api/connection-test-api.ts
New authenticated helpers: startConnectionTestSession, getConnectionTestResult, and resolveConnectionTestErrorMessage with typed response shapes.
Test button and handler in connection edit
features/admin.connections.v1/pages/connection-edit.tsx
Integrates resource endpoints, adds isTestingConnection state, and implements handleTestConnection that starts a session, opens the auth popup, and navigates to the test page with debugId.
Connection test results page
features/admin.connections.v1/pages/connection-test.tsx
Adds ConnectionTestPage (default export) with localStorage caching, auto-run via route state, manual rerun/poll flows, fetchResult, and UI tabs: ID Token (JWT), Claim Mappings, and Diagnosis.
Models and styles
features/admin.connections.v1/models/connection-test.ts, features/admin.connections.v1/models/connection.ts, features/admin.connections.v1/components/connection-test-styles.tsx
Defines connection-test result/diagnostic/mapped-claim interfaces and a set of themed styled components used by the test results UI.
Authorization and result popup pages
identity-apps-core/apps/authentication-portal/src/main/webapp/debugSuccess.jsp, identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp
Adds popup JSP pages for debug success (auto-close/countdown) and debug error (Back to Connection button) with extensible header/footer includes and client-side wiring.
Config state reformatting
features/admin.core.v1/store/reducers/config.ts
Initializes endpoints.debug and endpoints.debugResult in the reducer initial state and updates header year.
I18n keys and translations
modules/i18n/src/models/namespaces/authentication-provider-ns.ts, modules/i18n/src/translations/en-US/portals/authentication-provider.ts
Adds connectionTest localization keys and English translation strings used throughout the connection test UI.

Suggested reviewers:

  • DonOmalVindula
  • NipuniBhagya
  • pavinduLakshan

Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Changeset Required ❌ Error PR changes features/, modules/, and identity-apps-core/ directories with code changes but contains no .changeset/*.md files (only README.md and config.json). Add a .changeset/*.md file documenting all changed packages (@wso2is/admin.connections.v1, @wso2is/admin.core.v1, @wso2is/i18n, @wso2is/identity-apps-core) with appropriate version updates.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Introduce debug feature for FIdP' is specific and directly related to the main change (adding a debug flow for federated Identity Providers), making it clear and concise.
Description check ✅ Passed The PR description includes the Purpose section with clear explanation of the feature, Related Issues and PRs with valid links, and a complete Checklist section matching the template structure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • 🛠️ create changeset

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 16

🧹 Nitpick comments (6)
identity-apps-core/apps/authentication-portal/src/main/webapp/debugSuccess.jsp (1)

21-22: 💤 Low value

Unused imports Encode and StringEscapeUtils.

Neither symbol is referenced anywhere in the JSP. Drop them, or actually use Encode.forHtml(...) / Encode.forJavaScript(...) if there is text from request attributes that should be encoded (as needed in debugError.jsp).

🤖 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
`@identity-apps-core/apps/authentication-portal/src/main/webapp/debugSuccess.jsp`
around lines 21 - 22, Remove the unused imports Encode and StringEscapeUtils
from the JSP or replace their removal by applying proper encoding where needed;
specifically either delete the lines importing org.owasp.encoder.Encode and
org.apache.commons.text.StringEscapeUtils, or locate uses of request
attributes/output in debugSuccess.jsp and wrap them with Encode.forHtml(...) or
Encode.forJavaScript(...) (or StringEscapeUtils equivalents) to properly escape
user-controlled content before rendering.
features/admin.connections.v1/pages/connection-edit.tsx (1)

775-785: ⚡ Quick win

Localize the button label and confirm the icon choice.

The action button copy Test Connection and the <Icon name="flask"/> glyph are hardcoded:

  • Wrap the label in t(...) against an authenticationProvider: namespace key consistent with the surrounding tabs/buttons in this page.
  • For new UI introduced in this PR, the guideline is to use icons from @oxygen-ui/react-icons; semantic-ui-react's Icon name="flask" is being added fresh here. Either reuse an existing Oxygen icon (e.g., a "play/test" semantic) or keep semantic-ui-react consistent with the rest of the page's chrome and note the decision.

As per coding guidelines: "Use i18next via @wso2is/i18n with namespace-based keys in format 'namespace:path.to.key'" and "Import icons from @oxygen-ui/react-icons".

🤖 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 `@features/admin.connections.v1/pages/connection-edit.tsx` around lines 775 -
785, Replace the hardcoded button label and icon: wrap "Test Connection" with
the i18n translator t(...) using the authenticationProvider namespace (e.g.
t('authenticationProvider:connection.testButton')) where the SecondaryButton is
rendered (see SecondaryButton, handleTestConnection, isTestingConnection,
ConnectionsManagementUtils.isConnectorIdentityProvider); and replace the
Semantic UI Icon name="flask" with an icon imported from `@oxygen-ui/react-icons`
(choose an appropriate symbol such as PlayArrow or a test-related icon) to
follow the project's icon library guidelines, or if you intentionally keep
semantic-ui-react, note that decision in the PR description.
features/admin.connections.v1/pages/connection-test.tsx (4)

70-127: ⚡ Quick win

Replace raw window.localStorage with the shared LocalStorageUtils wrapper.

getCachedResult / cacheResult / clearCachedResult go directly to window.localStorage, which:

  • skips the standardized JSON envelope/error handling pattern used elsewhere in the console,
  • makes it harder to centrally clear test caches on tenant switch / sign-out,
  • requires the per-call typeof window === "undefined" guard you already write.

Use LocalStorageUtils (or SessionStorageUtils if these results shouldn't survive a browser restart — debug results probably shouldn't persist across sessions).

As per coding guidelines: "Use wrapper utilities from @wso2is/core/utils (SessionStorageUtils, LocalStorageUtils) instead of raw browser APIs".

🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx` around lines 70 -
127, Replace direct window.localStorage usage in getCachedResult, cacheResult,
and clearCachedResult with the shared wrapper (LocalStorageUtils or
SessionStorageUtils) so the standardized JSON envelope and centralized clearing
work; specifically, call the wrapper's getItem/setItem/removeItem (or
equivalent) using the same resultCacheKey, remove the manual
JSON.parse/JSON.stringify and the typeof window guards if the utility already
handles SSR, and update imports to pull LocalStorageUtils/SessionStorageUtils
from `@wso2is/core/utils` so getCachedResult, cacheResult, and clearCachedResult
use the utility API instead of window.localStorage.

52-55: 💤 Low value

Replace the inline SVG with an icon from @oxygen-ui/react-icons.

RotateIcon rolls its own SVG markup for a refresh/rotate glyph. Oxygen UI already exposes refresh-style icons that match the design system; importing one keeps stroke/fill behavior, accessibility, and sizing consistent with the rest of the console, and removes ~50 chars of inlined path data.

As per coding guidelines: "Import icons from @oxygen-ui/react-icons".

🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx` around lines 52 -
55, The RotateIcon component should stop inlining raw SVG and instead import and
render the matching icon from `@oxygen-ui/react-icons`; replace the RotateIcon
function body (the inline <svg> path) with a direct render of the appropriate
exported icon (e.g., Refresh/RefreshIcon from `@oxygen-ui/react-icons`), add any
sizing/props used elsewhere (width/height or fontSize) and accessibility props
(aria-hidden or role/title as required), and remove the large path data; keep
the same component name RotateIcon so call sites remain unchanged.

316-363: ⚡ Quick win

Tighten the result-status evaluation and consider extracting it.

A couple of small things in this effect:

  • Line 340: steps.accountLinkingStatus && steps.accountLinkingStatus !== "success" && steps.accountLinkingStatus !== "pending" is missing the parentheses that wrap the other three branches. JS precedence makes it functionally equivalent (&& binds tighter than ||), but the visual asymmetry is easy to misread when extending this list.
  • The whole result → {hasError, hasPartial, activeTab} derivation is pure and can be a memoized helper (useMemo) plus a tiny getStatusForResult(result) function in a utils/ file — easier to unit-test the partial/failure rules, and avoids re-deriving on unrelated re-renders.
  • setActiveTab(2) referencing the Diagnosis tab by numeric index is fragile; define enum ConnectionTestTab { ID_TOKEN = 0, CLAIM_MAPPINGS = 1, DIAGNOSIS = 2 } and use it both here and in <Tabs value={...}> below.
🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx` around lines 316 -
363, The effect that derives hasError/hasPartial/activeTab from result is
fragile and noisy: add missing parentheses around the accountLinkingStatus
condition in the hasStepError check to match the other branches (reference:
steps.accountLinkingStatus in the useEffect), extract the pure derivation into a
testable helper getStatusForResult(result) (or move to utils/) and memoize it
with useMemo to avoid recomputing on unrelated renders, and replace magic
numeric tab index 2 with a named enum ConnectionTestTab { ID_TOKEN = 0,
CLAIM_MAPPINGS = 1, DIAGNOSIS = 2 } then call
setActiveTab(ConnectionTestTab.DIAGNOSIS) (update Tabs value usage accordingly)
so the intent is clear and maintainable.

419-1078: 🏗️ Heavy lift

Localize all user-facing strings on this page.

A large number of strings rendered on this page are hardcoded English:

  • Tab labels: "ID Token", "Claim Mappings", "Diagnosis".
  • Empty/info states: "Unable to decode token or not a valid JWT.", "No claim mappings configured.", "No log information available.", "No diagnostic message provided.", "Timestamp unavailable".
  • Table headers: "Local Claim (URI)", "IDP Claim", "Value".
  • Status banner: "Test Failed" / "Test Passed Partially" / "Test Passed" and their descriptions.
  • Error/loading copy: "Error", "Retry", "Loading test results...", "Waiting for authentication to complete...", "Connection ID is missing. Cannot run test.", "Failed to fetch debug results.", "Failed to run test.", "Unable to retrieve test results for this session.", "No debug session id found...", "Test Results", "Results for the connection test session.", "Rerun Test".

Move all of these into the i18n namespaces (likely authenticationProvider: / console:develop.pages.idpTest.* to mirror the back-button key already present at line 986). Hardcoded copy here will leak through to translated builds and downstream consumers of @wso2is/i18n.

As per coding guidelines: "Use useTranslation() from react-i18next and access keys with format <namespace>:<path.to.key>".

🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx` around lines 419 -
1078, Page contains many hardcoded user strings; import and use useTranslation()
and replace all literal strings in renderResultsTabs/tabPanes (tab labels in
tabPanes with keys "id-token", "claim-mappings", "diagnosis"), decodeJWT alert
message, claim mappings empty state, table headers ("Local Claim (URI)","IDP
Claim","Value"), diagnosis messages ("No log information available.","No
diagnostic message provided.","Timestamp unavailable"), status banner texts and
descriptions, error/loading card labels ("Error","Retry","Loading test
results...","Waiting for authentication to complete..."), page title/description
and action button ("Test Results","Results for the connection test
session.","Rerun Test") with t("console:develop.pages.idpTest.<key>") or
appropriate authenticationProvider: keys; add matching keys to i18n namespaces,
keep existing data-testid and behavior unchanged, and ensure imports
(useTranslation) and use of t are added near the component (e.g., where
renderResultsTabs, decodeJWT, formatLogs are defined).
🤖 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 `@apps/console/src/configs/routes.tsx`:
- Line 591: The parent route object identityProviders currently has exact: false
which is inconsistent with the sibling applications route; update the routing so
matching for child routes is driven by the child definitions (e.g.,
identityProviderTest) not by making the parent non-exact: either change
identityProviders to exact: true to match applications, or if the test child
needs looser matching, set its own path/exact appropriately (adjust
identityProviderTest's path or exact flag to explicitly allow
/connections/:id/test) so parent exactness is consistent and route matching for
/connections/:id and /connections/:id/test behaves correctly.

In `@features/admin.connections.v1/configs/endpoints.ts`:
- Around line 41-42: The two endpoint fields debug and debugResult in the
endpoints config are identical causing redundancy and confusion; either remove
debugResult from the interface and all usages (since consumer pages use
resourceEndpoints.debug for both `${resourceEndpoints.debug}/idp` and
`${resourceEndpoints.debug}/${sid}/result`), or change debugResult to the actual
result URL pattern (e.g., include the `{sid}/result` segment) and update the
consumer pages to use resourceEndpoints.debug for the base and
resourceEndpoints.debugResult for the result retrieval; modify the endpoints
object (debug, debugResult) and the interface that defines them, and update any
consumer references to use the chosen single source of truth
(resourceEndpoints.debug or resourceEndpoints.debugResult) consistently.

In `@features/admin.connections.v1/pages/connection-edit.tsx`:
- Around line 696-759: handleTestConnection (and similar functions like
fetchResult/handleRunTests in connection-test.tsx) currently imports axios
directly, builds hardcoded tenant paths, uses English strings, and types the
catch as any; fix by extracting the POST call into a small api helper (e.g.,
api/connection-debug.ts) that uses AsgardeoSPAClient.getInstance().httpRequest
to call `${resourceEndpoints.debug}/idp`, replace manual path construction with
AppConstants.getPaths().get("IDP_TEST").replace(":id", connector.id) and use
AppConstants.getTenant() for tenant resolution, replace hardcoded messages
("Connection ID is not available.", "Debug session data is incomplete.") with
i18n keys via t("authenticationProvider:notifications.getIDP.*"), and change
catch (error: any) to catch (error: unknown) and narrow it with an
AxiosError/type-guard before reading response.data/message.

In `@features/admin.connections.v1/pages/connection-test.tsx`:
- Around line 384-417: renderCodeBlock and the surrounding table/accordion
renderers use many hardcoded hex colors and fixed spacings; replace those with
Oxygen UI styled components using MUI's styled API (e.g., create StyledCodeBlock
= styled(Box)(({ theme }) => ({ background: theme.palette.background.paper,
border: `1px solid ${theme.palette.divider}`, borderRadius:
theme.shape.borderRadius, padding: theme.spacing(1.5), fontFamily: 'monospace',
fontSize: 13, color: theme.palette.text.primary, whiteSpace: 'pre-wrap',
wordBreak: 'break-word' })) and similar StyledCard/StyledCell components, pull
success/warning/error backgrounds from
theme.palette.success.light/warning.light/error.light, use theme.spacing() for
margins and theme.shape.borderRadius for radii, and reduce inline sx usage to
only 2–3 layout props; update renderCodeBlock to return the styled component and
replace all hardcoded hex values in the table/accordion renderers with theme
palette references.
- Around line 263-313: Guard against null/undefined response shape and missing
debugId: replace direct access to response.data.metadata.authorizationUrl with
optional chaining (response.data?.metadata?.authorizationUrl) and validate
newDebugId after parsing (const newDebugId = response.data?.debugId) before
calling setDebugId, fetchResult or starting timers; if newDebugId is falsy, call
clearTimers(), setError with a helpful message and return early instead of
proceeding to open the popup or schedule fetches. Also ensure
popupInterval.current and fetchTimer.current are only created/cleared when
newDebugId is valid to avoid scheduling calls to fetchResult(undefined).
- Around line 368-376: Replace the hard-coded back URL in handleBackButtonClick
with the canonical path from AppConstants: construct the tenantDomain as
currently done, then call
history.push(AppConstants.getPaths().get("IDP_EDIT").replace("{tenantDomain}",
tenantDomain).replace("{id}", connectorId)) (or otherwise interpolate
connectorId into the returned pattern) so the code uses
AppConstants.getPaths().get("IDP_EDIT") instead of the literal
`/t/${tenantDomain}/console/connections/${connectorId}`; keep tenant extraction
and connectorId usage intact.
- Around line 488-498: The code mutates result.metadata.mappedClaims by calling
claimsArray.sort in place; fix by cloning before sorting (e.g., create
sortedClaims from [...(result?.metadata?.mappedClaims ?? [])].sort(...)) so the
original cached API array is not mutated, and add explicit types for the items
used in the comparator (introduce/consume a MappedClaim interface in models/ and
annotate claimsArray and the comparator parameters a: MappedClaim, b:
MappedClaim) to remove any/implicit typing.
- Around line 63-67: Replace the banned React.FC typing and any/implicit types:
declare a props interface named ConnectionTestPagePropsInterface that extends
IdentifiableComponentInterface and RouteComponentProps<RouteParams>, add a
ConnectionTestLocationStateInterface (debugId?: string; authorizationUrl?:
string) and type locationState as ConnectionTestLocationStateInterface |
undefined, change the component signature to
FunctionComponent<ConnectionTestPagePropsInterface> returning ReactElement
(ConnectionTestPage), add explicit useState generics for debugId (string|null),
result (Record<string, unknown>|null) or a more specific shape, error
(string|null), loading (boolean), hasError (boolean), hasPartial (boolean),
activeTab (number), autoRunTriggered (boolean), isStatusBannerVisible (boolean),
expandedDiagnosticLogs (number[]), and replace useRef<any>(null) with typed refs
like useRef<NodeJS.Timeout | null>(null) for popupInterval and fetchTimer (or
appropriate timer type).
- Around line 153-193: The useEffect in the test page currently calls
window.open(state.authorizationUrl, "_blank") from an async/automatic context
which triggers popup blocking; move the popup open logic into the user click
handler on the edit page (handleTestConnection) so the popup is opened
synchronously before calling history.push to the test page and pass only debugId
in location state; update the test page to only read debugId from locationState
and poll/fetchResult without re-opening the popup (identifiers: useEffect,
locationState, autoRunTriggered, debugId, fetchResult, popupInterval,
fetchTimer). Also make the authorizationUrl access defensive by changing
response.data.metadata.authorizationUrl to use optional chaining
(response.data?.metadata?.authorizationUrl) where the edit page reads it to
avoid NPEs.

In
`@identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp`:
- Around line 1-11: Replace the abridged license block with the full WSO2
Apache-2.0 boilerplate to match sibling JSPs; add the JSP page directive <%@
page language="java" contentType="text/html; charset=UTF-8" pageEncoding="UTF-8"
%> at the top to set response content-type/encoding; remove the unused import of
org.apache.commons.text.StringEscapeUtils (it is currently unused in this file);
and ensure AuthenticationEndpointUtil is available by confirming
includes/localize.jsp exposes it or by adding an explicit import for
AuthenticationEndpointUtil in this JSP if it is not provided transitively so
references to AuthenticationEndpointUtil compile.
- Around line 50-54: The "Back to Connection" button label is hardcoded; replace
the literal text with the i18n helper used elsewhere by calling
AuthenticationEndpointUtil.i18n(...) for that label so it is translated
consistently (update the button with id "backBtn" to render the localized string
via AuthenticationEndpointUtil.i18n with an appropriate key or the English text
as the default).
- Around line 85-121: The script currently calls window.close() unconditionally
(after wiring document.getElementById("backBtn").onclick) which will close popup
windows immediately and make the "Back to Connection" button unreachable; change
the code to only auto-close when the page is a popup (e.g., if window.opener is
truthy) or remove the auto-close entirely so the user can click the backBtn that
uses connectionUrl; also fix the catch block by guarding the
document.getElementById("json") reference (check for non-null before assigning
textContent) to avoid a secondary TypeError hiding the original error.
- Around line 90-105: The IIFE that computes baseUrl injects unescaped JSP
attributes (consoleUrlAttr, tenanted, serverUrl, tenantPrefix) into
single-quoted JS strings which risks XSS and syntax breakage; update each
attribute injection to use OWASP encoding (e.g., Encode.forJavaScript(<%=
request.getAttribute(...) %>)) like debugSuccess.jsp does, so consoleUrlAttr,
tenanted, serverUrl and tenantPrefix are safely escaped before use in the
baseUrl function; also remove the hardcoded "/t/carbon.super" fallback in the
baseUrl IIFE and make the fallback fail closed (e.g., return an empty string or
null) so tenanted deployments are not forced to that path.

In
`@identity-apps-core/apps/authentication-portal/src/main/webapp/debugSuccess.jsp`:
- Around line 119-254: This file contains two complete JSP pages; remove the
first duplicate document (the top half up to before the second <!doctype html>)
so only the intended debug-success page remains (keep the variant that uses
AuthenticationEndpointUtil.i18n("Debug Request was Completed"), the countdown
element with id="countdown", and the loadFunc() that decrements seconds from 2
and calls window.close()). Ensure you delete the earlier duplicated directives,
taglibs, header/footer includes, layout:main block, and the first loadFunc()
definition so there is only one set of page directives and one
loadFunc/countdown implementation.
- Around line 202-205: The i18n text is split around the countdown span which
breaks translation ordering and leaves the " s" unit unlocalized; replace the
fragmented message with a single localized template via
AuthenticationEndpointUtil.i18n that includes a placeholder for the seconds
(e.g., "You will be redirected to results page in {0}") or a full localized
format including the unit, then inject the numeric value into the placeholder
(keep the span id="countdown" for JS updates) so translators can
reorder/localize the unit correctly; ensure the template string used by
AuthenticationEndpointUtil.i18n is updated accordingly.
- Around line 235-252: The countdown currently decrements before the first paint
in loadFunc, so the initial "2" is skipped; fix by writing the initial value to
countdownElem before starting the interval (i.e., after obtaining countdownElem
set countdownElem.textContent = seconds), then start the setInterval as-is (or
alternatively move the decrement to after updating the DOM inside the interval)
so the displayed value (seconds) is visible for the first second; refer to
loadFunc, seconds, countdownElem, interval, and window.close when making the
change.

---

Nitpick comments:
In `@features/admin.connections.v1/pages/connection-edit.tsx`:
- Around line 775-785: Replace the hardcoded button label and icon: wrap "Test
Connection" with the i18n translator t(...) using the authenticationProvider
namespace (e.g. t('authenticationProvider:connection.testButton')) where the
SecondaryButton is rendered (see SecondaryButton, handleTestConnection,
isTestingConnection, ConnectionsManagementUtils.isConnectorIdentityProvider);
and replace the Semantic UI Icon name="flask" with an icon imported from
`@oxygen-ui/react-icons` (choose an appropriate symbol such as PlayArrow or a
test-related icon) to follow the project's icon library guidelines, or if you
intentionally keep semantic-ui-react, note that decision in the PR description.

In `@features/admin.connections.v1/pages/connection-test.tsx`:
- Around line 70-127: Replace direct window.localStorage usage in
getCachedResult, cacheResult, and clearCachedResult with the shared wrapper
(LocalStorageUtils or SessionStorageUtils) so the standardized JSON envelope and
centralized clearing work; specifically, call the wrapper's
getItem/setItem/removeItem (or equivalent) using the same resultCacheKey, remove
the manual JSON.parse/JSON.stringify and the typeof window guards if the utility
already handles SSR, and update imports to pull
LocalStorageUtils/SessionStorageUtils from `@wso2is/core/utils` so
getCachedResult, cacheResult, and clearCachedResult use the utility API instead
of window.localStorage.
- Around line 52-55: The RotateIcon component should stop inlining raw SVG and
instead import and render the matching icon from `@oxygen-ui/react-icons`; replace
the RotateIcon function body (the inline <svg> path) with a direct render of the
appropriate exported icon (e.g., Refresh/RefreshIcon from
`@oxygen-ui/react-icons`), add any sizing/props used elsewhere (width/height or
fontSize) and accessibility props (aria-hidden or role/title as required), and
remove the large path data; keep the same component name RotateIcon so call
sites remain unchanged.
- Around line 316-363: The effect that derives hasError/hasPartial/activeTab
from result is fragile and noisy: add missing parentheses around the
accountLinkingStatus condition in the hasStepError check to match the other
branches (reference: steps.accountLinkingStatus in the useEffect), extract the
pure derivation into a testable helper getStatusForResult(result) (or move to
utils/) and memoize it with useMemo to avoid recomputing on unrelated renders,
and replace magic numeric tab index 2 with a named enum ConnectionTestTab {
ID_TOKEN = 0, CLAIM_MAPPINGS = 1, DIAGNOSIS = 2 } then call
setActiveTab(ConnectionTestTab.DIAGNOSIS) (update Tabs value usage accordingly)
so the intent is clear and maintainable.
- Around line 419-1078: Page contains many hardcoded user strings; import and
use useTranslation() and replace all literal strings in
renderResultsTabs/tabPanes (tab labels in tabPanes with keys "id-token",
"claim-mappings", "diagnosis"), decodeJWT alert message, claim mappings empty
state, table headers ("Local Claim (URI)","IDP Claim","Value"), diagnosis
messages ("No log information available.","No diagnostic message
provided.","Timestamp unavailable"), status banner texts and descriptions,
error/loading card labels ("Error","Retry","Loading test results...","Waiting
for authentication to complete..."), page title/description and action button
("Test Results","Results for the connection test session.","Rerun Test") with
t("console:develop.pages.idpTest.<key>") or appropriate authenticationProvider:
keys; add matching keys to i18n namespaces, keep existing data-testid and
behavior unchanged, and ensure imports (useTranslation) and use of t are added
near the component (e.g., where renderResultsTabs, decodeJWT, formatLogs are
defined).

In
`@identity-apps-core/apps/authentication-portal/src/main/webapp/debugSuccess.jsp`:
- Around line 21-22: Remove the unused imports Encode and StringEscapeUtils from
the JSP or replace their removal by applying proper encoding where needed;
specifically either delete the lines importing org.owasp.encoder.Encode and
org.apache.commons.text.StringEscapeUtils, or locate uses of request
attributes/output in debugSuccess.jsp and wrap them with Encode.forHtml(...) or
Encode.forJavaScript(...) (or StringEscapeUtils equivalents) to properly escape
user-controlled content before rendering.
🪄 Autofix (Beta)

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.yml

Review profile: CHILL

Plan: Pro

Run ID: a9863fd1-3fc8-458a-9d76-d31afa11c96c

📥 Commits

Reviewing files that changed from the base of the PR and between ee4b60b and 44c322a.

📒 Files selected for processing (9)
  • apps/console/src/configs/routes.tsx
  • features/admin.connections.v1/configs/endpoints.ts
  • features/admin.connections.v1/models/endpoints.ts
  • features/admin.connections.v1/pages/connection-edit.tsx
  • features/admin.connections.v1/pages/connection-test.tsx
  • features/admin.core.v1/constants/app-constants.ts
  • features/admin.core.v1/store/reducers/config.ts
  • identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp
  • identity-apps-core/apps/authentication-portal/src/main/webapp/debugSuccess.jsp

Comment thread apps/console/src/configs/routes.tsx Outdated
Comment thread features/admin.connections.v1/configs/endpoints.ts Outdated
Comment thread features/admin.connections.v1/pages/connection-edit.tsx
Comment thread features/admin.connections.v1/pages/connection-test.tsx Outdated
Comment thread features/admin.connections.v1/pages/connection-test.tsx
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp (1)

113-116: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Hardcoded /t/carbon.super fallback is inappropriate for tenanted deployments.

The baseUrl IIFE falls back to window.location.origin + "/t/carbon.super" when no server attributes are provided. This forces multi-tenant deployments to the super-tenant path, which will break navigation for users in other tenants. Consider failing closed (return empty string or null) or using a tenant-aware fallback if available from the session context.

🛡️ Suggested guard
             return (window.location && window.location.origin
                 ? window.location.origin
-                : (window.location.protocol + "//" + window.location.hostname + (window.location.port ? ":" + window.location.port : "")))
-                + "/t/carbon.super";
+                : (window.location.protocol + "//" + window.location.hostname + (window.location.port ? ":" + window.location.port : "")));

Or if a tenant qualifier is always required, return null and guard connectionUrl construction below.

Based on learnings: Enforce consistent tenant URL pattern across the codebase: all tenant-scoped paths use /t//, including carbon.super as the super-tenant. Avoid assuming a root path for multi-tenant environments.

🤖 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 `@identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp`
around lines 113 - 116, The current baseUrl IIFE returns a hardcoded
"/t/carbon.super" fallback which forces multi-tenant users into the
super-tenant; update the IIFE (the baseUrl computation) to avoid returning
"/t/carbon.super" by default—either return null/empty string or derive the
tenant qualifier from a session/context variable if available, and update any
downstream usage (e.g., connectionUrl construction) to guard against a
null/empty baseUrl; specifically modify the baseUrl IIFE expression so it fails
closed (null/""), or uses a tenant-aware value from sessionTenant (or
equivalent) instead of the hardcoded "/t/carbon.super", and ensure code that
builds connectionUrl checks for the new null/empty value before concatenation.
🧹 Nitpick comments (4)
features/admin.connections.v1/pages/connection-test.tsx (1)

771-771: ⚡ Quick win

Replace any with the proper diagnostic log interface.

Line 771 uses any for the log parameter in the diagnostics.map callback, which violates the no-any guideline. Use the already-defined ConnectionTestDiagnosticLogInterface instead.

♻️ Proposed fix
-                                            { diagnostics.map((log: any, idx: number) => {
+                                            { diagnostics.map((log: ConnectionTestDiagnosticLogInterface, idx: number) => {

As per coding guidelines: "Never use any; use proper types or unknown with type guards".

🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx` at line 771, The map
callback uses an `any` type for `log`; replace it with the specific interface
`ConnectionTestDiagnosticLogInterface` by changing the signature in the
diagnostics.map call (e.g., `diagnostics.map((log:
ConnectionTestDiagnosticLogInterface, idx: number) => { ... })`) and ensure
`ConnectionTestDiagnosticLogInterface` is imported or available in this module;
also update any local destructuring/uses of `log` to match the interface fields
and remove other `any` usages tied to this mapping.
identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp (1)

22-22: 💤 Low value

Remove unused import.

StringEscapeUtils is imported on line 22 but never used in this file. Remove it to keep the imports clean.

♻️ Proposed fix
 <%@ page import="org.owasp.encoder.Encode" %>
-<%@ page import="org.apache.commons.text.StringEscapeUtils" %>
 <%@ page language="java" contentType="text/html; charset=UTF-8" pageEncoding="UTF-8"%>
🤖 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 `@identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp`
at line 22, Remove the unused import of StringEscapeUtils from debugError.jsp:
locate the page/import directive that references StringEscapeUtils (the import
of org.apache.commons.lang3.StringEscapeUtils or similar) and delete that import
line so the JSP no longer imports an unused class; keep the other directives
like the jsp:directive.include for includes/init-url.jsp unchanged.
features/admin.connections.v1/pages/connection-edit.tsx (1)

769-779: ⚡ Quick win

Replace data-testid with data-componentid and internationalize the button label.

Line 771 uses the deprecated data-testid attribute; per coding guidelines, new components should use data-componentid. Additionally, the "Test Connection" label on line 777 is hardcoded English and should be wrapped in t(...) to support i18n.

♻️ Proposed fix
                 action={ ConnectionsManagementUtils.isConnectorIdentityProvider(connector) ? (
                     <SecondaryButton
-                        data-testid="idp-test-connection-button"
+                        data-componentid="idp-test-connection-button"
                         onClick={ handleTestConnection }
                         loading={ isTestingConnection }
                         disabled={ isTestingConnection }
                     >
                         <Icon name="flask"/>
-                        Test Connection
+                        { t("authenticationProvider:buttons.testConnection") }
                     </SecondaryButton>
                 ) : null }

Based on learnings: In the identity-apps repository, prefer using the data-componentid attribute for component identification in tests. The data-testid attribute is deprecated.

🤖 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 `@features/admin.connections.v1/pages/connection-edit.tsx` around lines 769 -
779, Replace the deprecated data-testid with data-componentid on the
SecondaryButton used in the
ConnectionsManagementUtils.isConnectorIdentityProvider branch and
internationalize its label by wrapping "Test Connection" with the i18n
translator (t(...)); update the attribute on the SecondaryButton rendered in
that branch (the element using handleTestConnection, isTestingConnection and
loading/disabled props) and ensure the translator function (e.g., t from
useTranslation) is in scope or imported so the button label is rendered via
t('...') instead of a hardcoded string.
features/admin.connections.v1/api/connection-test-api.ts (1)

92-99: ⚡ Quick win

Strengthen the type guard by checking the isAxiosError value.

The type guard checks for the presence of the isAxiosError property but not its value. Axios sets isAxiosError: true on error instances, so verifying the boolean value would make the guard more precise.

♻️ Suggested improvement
 const isConnectionTestAxiosError = (
     error: unknown
 ): error is AxiosError<ConnectionTestErrorResponseInterface> => {
     const candidate: AxiosError<ConnectionTestErrorResponseInterface> =
         error as AxiosError<ConnectionTestErrorResponseInterface>;
 
-    return Boolean(candidate && typeof candidate === "object" && "isAxiosError" in candidate);
+    return Boolean(
+        candidate &&
+        typeof candidate === "object" &&
+        "isAxiosError" in candidate &&
+        candidate.isAxiosError === true
+    );
 };
🤖 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 `@features/admin.connections.v1/api/connection-test-api.ts` around lines 92 -
99, The type guard isConnectionTestAxiosError currently only checks for the
presence of isAxiosError; update it to verify the property value is true to make
the guard precise (e.g., after casting to candidate:
AxiosError<ConnectionTestErrorResponseInterface>, ensure candidate && typeof
candidate === "object" && candidate.isAxiosError === true). Keep the same return
type and use the existing candidate variable and
AxiosError<ConnectionTestErrorResponseInterface> type in the condition.
🤖 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 `@features/admin.connections.v1/api/connection-test-api.ts`:
- Around line 24-26: The httpClient initialization binds
AsgardeoSPAClient.getInstance().httpRequest twice which is redundant; update the
initialization of httpClient to bind httpRequest only once to the
AsgardeoSPAClient instance (use
AsgardeoSPAClient.getInstance().httpRequest.bind(AsgardeoSPAClient.getInstance()))
so the httpRequest method has the correct this context without the extra .bind()
call; reference AsgardeoSPAClient, httpRequest, and the httpClient variable when
making the change.

---

Duplicate comments:
In
`@identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp`:
- Around line 113-116: The current baseUrl IIFE returns a hardcoded
"/t/carbon.super" fallback which forces multi-tenant users into the
super-tenant; update the IIFE (the baseUrl computation) to avoid returning
"/t/carbon.super" by default—either return null/empty string or derive the
tenant qualifier from a session/context variable if available, and update any
downstream usage (e.g., connectionUrl construction) to guard against a
null/empty baseUrl; specifically modify the baseUrl IIFE expression so it fails
closed (null/""), or uses a tenant-aware value from sessionTenant (or
equivalent) instead of the hardcoded "/t/carbon.super", and ensure code that
builds connectionUrl checks for the new null/empty value before concatenation.

---

Nitpick comments:
In `@features/admin.connections.v1/api/connection-test-api.ts`:
- Around line 92-99: The type guard isConnectionTestAxiosError currently only
checks for the presence of isAxiosError; update it to verify the property value
is true to make the guard precise (e.g., after casting to candidate:
AxiosError<ConnectionTestErrorResponseInterface>, ensure candidate && typeof
candidate === "object" && candidate.isAxiosError === true). Keep the same return
type and use the existing candidate variable and
AxiosError<ConnectionTestErrorResponseInterface> type in the condition.

In `@features/admin.connections.v1/pages/connection-edit.tsx`:
- Around line 769-779: Replace the deprecated data-testid with data-componentid
on the SecondaryButton used in the
ConnectionsManagementUtils.isConnectorIdentityProvider branch and
internationalize its label by wrapping "Test Connection" with the i18n
translator (t(...)); update the attribute on the SecondaryButton rendered in
that branch (the element using handleTestConnection, isTestingConnection and
loading/disabled props) and ensure the translator function (e.g., t from
useTranslation) is in scope or imported so the button label is rendered via
t('...') instead of a hardcoded string.

In `@features/admin.connections.v1/pages/connection-test.tsx`:
- Line 771: The map callback uses an `any` type for `log`; replace it with the
specific interface `ConnectionTestDiagnosticLogInterface` by changing the
signature in the diagnostics.map call (e.g., `diagnostics.map((log:
ConnectionTestDiagnosticLogInterface, idx: number) => { ... })`) and ensure
`ConnectionTestDiagnosticLogInterface` is imported or available in this module;
also update any local destructuring/uses of `log` to match the interface fields
and remove other `any` usages tied to this mapping.

In
`@identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp`:
- Line 22: Remove the unused import of StringEscapeUtils from debugError.jsp:
locate the page/import directive that references StringEscapeUtils (the import
of org.apache.commons.lang3.StringEscapeUtils or similar) and delete that import
line so the JSP no longer imports an unused class; keep the other directives
like the jsp:directive.include for includes/init-url.jsp unchanged.
🪄 Autofix (Beta)

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.yml

Review profile: CHILL

Plan: Pro

Run ID: 2b44b838-3aea-4386-9f29-74cdce12c1f5

📥 Commits

Reviewing files that changed from the base of the PR and between 44c322a and 0f9e7db.

📒 Files selected for processing (10)
  • apps/console/src/configs/routes.tsx
  • features/admin.connections.v1/api/connection-test-api.ts
  • features/admin.connections.v1/components/connection-test-styles.tsx
  • features/admin.connections.v1/configs/endpoints.ts
  • features/admin.connections.v1/models/connection-test.ts
  • features/admin.connections.v1/models/connection.ts
  • features/admin.connections.v1/pages/connection-edit.tsx
  • features/admin.connections.v1/pages/connection-test.tsx
  • identity-apps-core/apps/authentication-portal/src/main/webapp/debugError.jsp
  • identity-apps-core/apps/authentication-portal/src/main/webapp/debugSuccess.jsp
✅ Files skipped from review due to trivial changes (1)
  • features/admin.connections.v1/components/connection-test-styles.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • features/admin.connections.v1/configs/endpoints.ts

Comment thread features/admin.connections.v1/api/connection-test-api.ts Outdated
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
features/admin.connections.v1/pages/connection-test.tsx (1)

898-921: 💤 Low value

Use i18n translation keys and data-componentid attribute.

Several UI strings are hardcoded and the component uses deprecated data-testid instead of data-componentid:

  1. Replace hardcoded strings with translation keys:

    • Line 898: pageTitle="Test Results" → use t("...")
    • Line 900: description="Results for the connection test session." → use t("...")
    • Line 915: Rerun Test → use t("...")
  2. Replace data-testid with data-componentid throughout (lines 902, 913, 921, 947, 958, 970, 980).

Based on learnings: "prefer using the data-componentid attribute... data-testid is deprecated."

🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx` around lines 898 -
921, Replace hardcoded UI strings with i18n lookups and swap deprecated
data-testid attributes for data-componentid: change pageTitle="Test Results" to
pageTitle={t("console:develop.pages.idpTest.testResults.pageTitle", "Test
Results")}, description="Results for the connection test session." to
description={t("console:develop.pages.idpTest.testResults.description", "Results
for the connection test session.")}, and the Button label "Rerun Test" inside
the action prop to t("console:develop.pages.idpTest.testResults.rerunButton",
"Rerun Test"); also replace all data-testid props shown in this JSX (the
backButton object key, the Button prop, and the outer PageLayout prop currently
using `${testId}-...`) with data-componentid preserving the same identifier
string pattern (e.g. data-componentid={`${testId}-back-button`} etc.) so the
Back button, rerun Button, page layout and other occurrences use
data-componentid instead of data-testid.
🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx`:
- Line 770: Replace the use of the any type in the diagnostics.map callback by
typing the log parameter as ConnectionTestDiagnosticLogInterface; update the
arrow function signature where diagnostics.map((log: any, idx: number) => { ...)
to diagnostics.map((log: ConnectionTestDiagnosticLogInterface, idx: number) => {
...) so the component uses the imported ConnectionTestDiagnosticLogInterface for
proper typing.

In `@features/admin.core.v1/store/reducers/config.ts`:
- Around line 413-420: The object literal commonConfigReducerInitialState has
missing closing braces/commas around nested properties (e.g., after the users
block before enterpriseSAML and before microsoft) causing syntax errors; fix by
closing each nested object with a } and adding a trailing comma between sibling
properties (ensure users: null is followed by }, or if users is an object it is
closed, then add , before enterpriseSAML, and likewise close the block preceding
microsoft and add a comma), validate the whole commonConfigReducerInitialState
for matching braces and commas, and re-run the TypeScript/lint build to confirm
the syntax errors are resolved.
- Around line 154-155: The config reducer is missing a closing brace and comma
after the endpoints object, causing the object containing workflows and adminApp
to be malformed; close the endpoints object (add the missing "}" ), ensure
there's a comma after workflows: "" if needed, and confirm adminApp: { ... } is
correctly nested inside the top-level config object (check symbols: endpoints,
workflows, and adminApp in the reducer) so the TypeScript syntax is valid and
compilation errors are resolved.

---

Nitpick comments:
In `@features/admin.connections.v1/pages/connection-test.tsx`:
- Around line 898-921: Replace hardcoded UI strings with i18n lookups and swap
deprecated data-testid attributes for data-componentid: change pageTitle="Test
Results" to pageTitle={t("console:develop.pages.idpTest.testResults.pageTitle",
"Test Results")}, description="Results for the connection test session." to
description={t("console:develop.pages.idpTest.testResults.description", "Results
for the connection test session.")}, and the Button label "Rerun Test" inside
the action prop to t("console:develop.pages.idpTest.testResults.rerunButton",
"Rerun Test"); also replace all data-testid props shown in this JSX (the
backButton object key, the Button prop, and the outer PageLayout prop currently
using `${testId}-...`) with data-componentid preserving the same identifier
string pattern (e.g. data-componentid={`${testId}-back-button`} etc.) so the
Back button, rerun Button, page layout and other occurrences use
data-componentid instead of data-testid.
🪄 Autofix (Beta)

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.yml

Review profile: CHILL

Plan: Pro

Run ID: 4ac56e99-0614-4f16-90f4-a186b07db8e6

📥 Commits

Reviewing files that changed from the base of the PR and between 0f9e7db and 27bf651.

📒 Files selected for processing (11)
  • apps/console/src/configs/routes.tsx
  • features/admin.connections.v1/api/connection-test-api.ts
  • features/admin.connections.v1/components/connection-test-styles.tsx
  • features/admin.connections.v1/configs/endpoints.ts
  • features/admin.connections.v1/models/connection-test.ts
  • features/admin.connections.v1/models/connection.ts
  • features/admin.connections.v1/models/endpoints.ts
  • features/admin.connections.v1/pages/connection-edit.tsx
  • features/admin.connections.v1/pages/connection-test.tsx
  • features/admin.core.v1/constants/app-constants.ts
  • features/admin.core.v1/store/reducers/config.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • features/admin.connections.v1/models/endpoints.ts
  • features/admin.core.v1/constants/app-constants.ts
  • features/admin.connections.v1/models/connection.ts
  • apps/console/src/configs/routes.tsx
  • features/admin.connections.v1/pages/connection-edit.tsx
  • features/admin.connections.v1/components/connection-test-styles.tsx

Comment thread features/admin.connections.v1/pages/connection-test.tsx Outdated
Comment thread features/admin.core.v1/store/reducers/config.ts Outdated
Comment thread features/admin.core.v1/store/reducers/config.ts Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.70%. Comparing base (c8c969f) to head (e9a7355).
⚠️ Report is 22 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10277   +/-   ##
=======================================
  Coverage   55.70%   55.70%           
=======================================
  Files          42       42           
  Lines        1016     1016           
  Branches      254      231   -23     
=======================================
  Hits          566      566           
  Misses        450      450           
Flag Coverage Δ
@wso2is/core 55.70% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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 `@features/admin.connections.v1/pages/connection-test.tsx`:
- Around line 129-131: The cached ConnectionTestResult is being read/written
using local persistence (getCachedResult/LocalStorage), which persists sensitive
debug payloads across browser restarts; change the cache to use
SessionStorageUtils instead so results persist only for the session. Update any
getCachedResult/setCachedResult helpers (or the initialization of result via
getCachedResult and the places that write the cache around the
ConnectionTestResultInterface) to use SessionStorageUtils APIs (get/set/remove)
rather than LocalStorageUtils or raw localStorage; ensure the same change is
applied for the other cache usage referenced near the component (the second
cached block around the other initialization at lines 284-287) and keep the
debugId state handling unchanged.
- Around line 688-695: The current empty-state check is wrong because steps
always has fixed keys; update the condition used to render the "no logs" empty
state in connection-test.tsx to verify whether any step values are actually
populated (e.g., use Object.values(steps).every(v => !v) or check
diagnostics.length === 0) instead of Object.keys(steps).length === 0; apply the
same fix for the second occurrence around the block that references the same
steps/diagnostics logic (the second occurrence you noted around lines 795-803),
ensuring you reference the steps object, stepStatus, metadataObj and the
diagnostics array when deciding to show the empty-state alert.
- Around line 320-323: The handleRunTests() flow allows overlapping runs because
loading is only set later in fetchResult()/pollForResult(), so buttons remain
enabled and multiple startConnectionTestSession() calls/timers can be created;
fix by setting the loading state at the very beginning of handleRunTests(),
immediately clearing any existing timers (e.g., those used by
pollForResult()/fetchResult()) before calling
startConnectionTestSession(connectorId, resourceEndpoints), and ensure every
early-return or catch path resets loading to false and cleans timers so stale
timers cannot update the page; apply the same pattern to the other similar
blocks (around lines where startConnectionTestSession is called, and the
pollForResult()/fetchResult logic).
- Around line 276-289: The poll handler currently falls through and treats
DEBUG_STATUS_INCOMPLETE as a finished result when attempt === POLL_MAX_ATTEMPTS;
change the logic in the polling branch (where DEBUG_STATUS_INCOMPLETE,
POLL_MAX_ATTEMPTS, pollForResult are used) so that if response.data?.status ===
DEBUG_STATUS_INCOMPLETE AND attempt >= POLL_MAX_ATTEMPTS you do NOT call
setResult/cacheResult as-is; instead set an explicit timeout/error result (e.g.
setResult({ status: 'TIMEOUT', message: 'Polling timed out' }) or flip an error
state), call clearCachedResult(), setIsStatusBannerVisible(true) to show an
error/timeout banner, avoid caching the incomplete payload, and ensure loading
is set to false; leave normal pollForResult behavior unchanged when attempt <
POLL_MAX_ATTEMPTS.
🪄 Autofix (Beta)

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.yml

Review profile: CHILL

Plan: Pro

Run ID: 7754f041-3ded-40cf-9100-78c4d74abd83

📥 Commits

Reviewing files that changed from the base of the PR and between c9cd0b3 and 6170a93.

📒 Files selected for processing (2)
  • features/admin.connections.v1/pages/connection-test.tsx
  • features/admin.core.v1/store/reducers/config.ts

Comment thread features/admin.connections.v1/pages/connection-test.tsx Outdated
Comment thread features/admin.connections.v1/pages/connection-test.tsx Outdated
Comment thread features/admin.connections.v1/pages/connection-test.tsx Outdated
Comment thread features/admin.connections.v1/pages/connection-test.tsx Outdated
* Add marketing consent view

* Update text

* Bug fixes

* Update MyAccount UI

* Add changeset 🦋

* Fix review comments

* Update knip.json

* Refactor

* Show policy elements in suborgs

* Fix review comments

* Fix review comments

* UI improvement

* Enable suborg pages

* Rename to preference management

* Fix review comments

* Fix review comments
@LinukaAr LinukaAr changed the base branch from master to feature-connection-debug May 29, 2026 05:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants